Bug 190070 - Review Request: par2cmdline
Review Request: par2cmdline
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jason Tibbitts
Fedora Package Reviews List
:
: 183067 (view as bug list)
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-04-27 06:19 EDT by Laurent Rineau
Modified: 2009-06-30 22:37 EDT (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-05-09 06:17:59 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
tibbs: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Laurent Rineau 2006-04-27 06:19:27 EDT
Spec URL: http://www.di.ens.fr/~rineau/Fedora/par2cmdline.spec
SRPM URL: http://www.di.ens.fr/~rineau/Fedora/par2cmdline-0.4-5.src.rpm
Description: par2cmdline is a program for creating and using PAR2 files to
detect damage in data files and repair them if necessary. It can be used with
any kind of file.
Comment 1 Jason Tibbitts 2006-04-27 10:44:22 EDT
Issues:
spectool cannot fetch the upstream source; your Source: URL is wrong.  I think
it should be http://dl.sourceforge.net/parchive/%{name}-%{version}.tar.gz, which
I'll assume is the proper upstream.

To help those of us who won't understand why we would need this software, could
you perhaps include a quick description of a PAR2 file in %description?

Please use the recommended BuildRoot:
%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)

Please remove Distribution: SuSE 9.1.

I'm not sure about your Obsoletes: and Provides:, but I'll assume you have some
previous package history that requires this.  I'll ask the list for a bit of
guidance.

rpmlint complains:

E: par2cmdline-debuginfo script-without-shellbang
/usr/src/debug/par2cmdline-0.4/par2repairersourcefile.h
E: par2cmdline-debuginfo script-without-shellbang
/usr/src/debug/par2cmdline-0.4/par2repairer.cpp
E: par2cmdline-debuginfo script-without-shellbang
/usr/src/debug/par2cmdline-0.4/galois.h
E: par2cmdline-debuginfo script-without-shellbang
/usr/src/debug/par2cmdline-0.4/par2repairersourcefile.cpp
E: par2cmdline-debuginfo script-without-shellbang
/usr/src/debug/par2cmdline-0.4/par1repairer.cpp

You should remove the executable bits from these files in %build; otherwise RPM
thinks they're executables and sticks them in the debuginfo package.

Review:
* package meets naming and packaging guidelines.
X specfile is properly named but the preamble needs minor cleanup.  %prep and
below look good.
* license field matches the actual license.
* license is open source-compatible and is included in the package as %doc.
* source files match upstream:
   1551b63e57e3c232254dc62073b723a9  par2cmdline-0.4.tar.gz
   1551b63e57e3c232254dc62073b723a9  par2cmdline-0.4.tar.gz-srpm
* BuildRequires are proper.
* package builds in mock (development, x86_64).
X rpmlint has a few complaints
X final requires are sane; final provides 
* no shared libraries are present.
* package is not relocatable.
* creates no non-%doc directories.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* %clean is present.
O %check not present; no test suite upstream.
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* no headers.
* no pkgconfig files.
* no libtool .la droppings.
* not a GUI app.
Comment 2 Laurent Rineau 2006-04-27 12:58:03 EDT
Updated:
  Spec URL: http://www.di.ens.fr/~rineau/Fedora/par2cmdline.spec
  SRPM URL: http://www.di.ens.fr/~rineau/Fedora/par2cmdline-0.4-6.src.rpm

(In reply to comment #1)
> Issues:
> spectool cannot fetch the upstream source; your Source: URL is wrong.

Fixed, I think. I cannot reach dl.sourceforge.net at the present time. :-(

> you perhaps include a quick description of a PAR2 file in %description?

I modified it:
"par2cmdline is a program for creating and using PAR2 files. PAR2 files are
usually published in binary newsgroups, on Usenet. They apply the
data-recovery capability concepts of RAID-like systems to the posting and
recovery of multi-part archives. On Usenet, PAR2 files are posted together
with multi-part archives. par2cmdline can detect and repair dammaged files
using the corresponding PAR2 files."

Perhaps somebody can help me to correct my english. I declare the hunt of 
frenchisms opened! ;)

(I personnaly use it to repair files from binaries of Lost, season 2, which 
will not be on french TV until next year. But do not tell to anybody!)


> Please use the recommended BuildRoot:
> %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)

Done.

> Please remove Distribution: SuSE 9.1.

Sorry! Done. As you can see, I have adapted an existing (and trivial) spec 
file.

> I'm not sure about your Obsoletes: and Provides:, but I'll assume you 
> have some previous package history that requires this.  I'll ask the list
> for a bit of guidance.

I do not understand. parchive-1.1.4 exists in Fedora Extras 4. See
ftp://download.fedora.redhat.com/pub/fedora/linux/extras/4/SRPMS/parchive-1.1-4.src.rpm

par2cmdline provides the same functionalities as parchive, and has a 
symlink /usr/bin/par->par2. That's why I made this couple of 
Obsoletes:/Provides: Perhaps I am wrong. I must admit that I am not totally 
used with such a trick.

> rpmlint complains:
> You should remove the executable bits from these files in %build;

Done.
Comment 3 Jason Tibbitts 2006-04-27 20:31:41 EDT
Thanks for fixing things up.  I'll just respond point by point:

The source URL is good; Sourceforge often fails to respond for me, but things
usually start to work if you try long enough.

Let me try to fix up the %description a bit, using what's in the README:
----
par2cmdline is a program for creating and using PAR2 files to detect damage in
data files and repair them if necessary.  PAR2 files are usually published in
binary newsgroups on Usenet; they apply the data-recovery capability concepts of
RAID-like systems to the posting and recovery of multi-part archives.
----

BuildRoot: is now good.
Distribution: SuSE 9.1 is gone.
The problem with Obsoletes: is that you's behavior can be counter-intuitive, but
what you have here is fine.

rpmlint is now silent.

The only remaining issue is that I didn't notice the test suite upstream (which
was dumb of me since it's pretty obvious).  Can you add this after %clean:

%check
make check-TESTS

to run the test suite?  I checked that it passes on i386 and x86_64.

APPROVED.  Just add the %check bit when you check in.
Comment 4 Laurent Rineau 2006-04-28 05:07:44 EDT
(
Update:
  Spec URL: http://www.di.ens.fr/~rineau/Fedora/par2cmdline.spec
  SRPM URL: http://www.di.ens.fr/~rineau/Fedora/par2cmdline-0.4-7.src.rpm
)

Thank you Jason for your review!

I am waiting now for my account creation approval. 

(...)

Having read the guide again, it appears to me that I forgot one step: the 
sponsoring! :-( This my first review request for Fedora Extras, and I should 
have been reviewed by a sponsor! Jason, you did a good job with this review, 
however.
I have added FE-NEEDSPONSOR to blockers. I hope this will unblock the 
situation.
Comment 5 Jason Tibbitts 2006-05-07 19:08:31 EDT
I will sponsor you; you can go ahead and set up your Fedora account.
Comment 6 Laurent Rineau 2006-05-09 06:17:59 EDT
par2cmdline-0.4-8.src.rpm eventually builds correctly.

The diff between releases 7 and 8 is a patch that removes warnings on all 
platforms, and fix a compilation error on ppc (see CVS for details).

I think this bug can be closed. Thank you Jason for your support.

Perhaps will I ask you a few question about branches and things like that...
Comment 7 Laurent Rineau 2006-05-31 08:25:23 EDT
*** Bug 183067 has been marked as a duplicate of this bug. ***
Comment 8 Erik van Pienbroek 2009-06-30 05:41:13 EDT
As per bug 508772 the ownership of this package was transferred to me

Package Change Request
======================
Package Name: par2cmdline
New Branches: EL-5
Owners: epienbro konradm
Comment 9 Jason Tibbitts 2009-06-30 22:37:57 EDT
CVS done.

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