Bug 283461 - Review Request: qpxtool - CD/DVD Quality check tool
Review Request: qpxtool - CD/DVD Quality check tool
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-09-08 07:39 EDT by Adel Gadllah
Modified: 2007-11-30 17:12 EST (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-09-24 14:36:26 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
mtasaka: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Adel Gadllah 2007-09-08 07:39:50 EDT
Spec URL: http://tgmweb.at/gadllah/qpxtool.spec
SRPM URL: http://tgmweb.at/gadllah/qpxtool-0.6.1-1.fc7.src.rpm
Description:
QPxTool is the linux way to get full control over your CD/DVD drives.
It is the Open Source Solution which intends to give you access to all
available Quality Checks (Q-Checks) on written and blank media, that
are available for your drive. This will help you to find the right media
and the optimized writing speed for your hardware, which will increase
the chance for a long data lifetime.
Comment 1 Till Maas 2007-09-08 19:27:39 EDT
Use:
Source0: http://downloads.sourceforge.net/%{name}/%{name}-%{version}.tar.bz2
See:
http://fedoraproject.org/wiki/Packaging/SourceURL#head-e27982f18a3bfd26b5b6ecbee113d2d8f3f006f2


for -devel subpackge:
Requires: %{name} = %{version}
should be
Requires: %{name} = %{version}-%{release}


%files devel is missing a %defattr(-,root,root,-)


the cp invocations should be used with "-p" to preserve the timestamp.


The Makefile also defines INCDIR (%{_includedir}), which you do not adjust with sed.


the manpage is not installed with a preserved timestamp, missing "-p" in the
install invocation in man/Makefile (line 10), maybe upstream would change this.


In case you did not know:
you should be able to use make install PREFIX=%{_prefix} LIBDIR=%{_libdir} ...
instead of the sed magic. 

Also you can use all sed expressions at once:
sed -i -e 's!/usr/local!%{_prefix}!g' \
 -e 's!\$(PREFIX)/bin!%{_sbindir}!g' \
 -e 's!\$(PREFIX)/lib!%{_libdir}!g' \
 -e 's!\$(PREFIX)/man!%{_mandir}!g' Makefile
Comment 2 Adel Gadllah 2007-09-09 03:18:48 EDT
(In reply to comment #1)
> Use:
> Source0: http://downloads.sourceforge.net/%{name}/%{name}-%{version}.tar.bz2
> See:
>
http://fedoraproject.org/wiki/Packaging/SourceURL#head-e27982f18a3bfd26b5b6ecbee113d2d8f3f006f2

ok

> 
> for -devel subpackge:
> Requires: %{name} = %{version}
> should be
> Requires: %{name} = %{version}-%{release}

ok

> 
> %files devel is missing a %defattr(-,root,root,-)

ok

> 
> the cp invocations should be used with "-p" to preserve the timestamp.
> 
> 
> The Makefile also defines INCDIR (%{_includedir}), which you do not adjust
with sed.

ok

> 
> the manpage is not installed with a preserved timestamp, missing "-p" in the
> install invocation in man/Makefile (line 10), maybe upstream would change this.

ok fixed with a small patch for now.

> 
> In case you did not know:
> you should be able to use make install PREFIX=%{_prefix} LIBDIR=%{_libdir} ...
> instead of the sed magic. 

I know but this would be a long "make line" so I preferd doing it this way.

> Also you can use all sed expressions at once:
> sed -i -e 's!/usr/local!%{_prefix}!g' \
>  -e 's!\$(PREFIX)/bin!%{_sbindir}!g' \
>  -e 's!\$(PREFIX)/lib!%{_libdir}!g' \
>  -e 's!\$(PREFIX)/man!%{_mandir}!g' Makefile

same here I think that this is easier to read

New fixed package:

http://tgmweb.at/gadllah/qpxtool.spec
http://tgmweb.at/gadllah/qpxtool-0.6.1-2.fc7.src.rpm
Comment 3 Mamoru TASAKA 2007-09-20 09:33:50 EDT
Rebuild fails on dist-f8.

http://koji.fedoraproject.org/koji/taskinfo?taskID=167410
Comment 5 Mamoru TASAKA 2007-09-22 12:25:04 EDT
Almost clean.

For 0.6.1-3:
* SourceURL
  - For sourceforge source, please follow:
    http://fedoraproject.org/wiki/Packaging/SourceURL

* Desktop category
----------------------------------------------------
Categories=Application;System;X-Fedora;
----------------------------------------------------
  - Both "Application" "X-Fedora" are deprecated and should
    be removed.
Comment 6 Adel Gadllah 2007-09-22 12:48:02 EDT
I fixed both in 0.6.1-4:
http://tgmweb.at/gadllah/qpxtool.spec
http://tgmweb.at/gadllah/qpxtool-0.6.1-4.fc7.src.rpm
Comment 7 Mamoru TASAKA 2007-09-22 13:15:36 EDT
Please change the category to
-------------------------------------------------
Categories=System;
-------------------------------------------------
i.e. Each category must end with semicolon (although when
     installed with desktop-file-install, desktop-file-install
     automatically fixes it and adds semicolon at last).

-------------------------------------------------
   This package (qpxtool) is APPROVED by me
-------------------------------------------------
Comment 8 Adel Gadllah 2007-09-22 13:36:09 EDT
ok fixed in  0.6.1-5, thx for the review.
--

New Package CVS Request
=======================
Package Name: qpxtool
Short Description: CD/DVD Quality check tool
Owners: drago01
Branches: F-7
Cvsextras Commits: no
Comment 9 Kevin Fenzi 2007-09-24 12:28:45 EDT
cvs done.
Comment 10 Mamoru TASAKA 2007-09-24 14:09:04 EDT
Please close this bug when rebuild is done.

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