Bug 1111242

Summary: Review Request: perl-App-s2p - Convert sed script to Perl program
Product: [Fedora] Fedora Reporter: Jitka Plesnikova <jplesnik>
Component: Package ReviewAssignee: Petr Pisar <ppisar>
Status: CLOSED RAWHIDE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: package-review, ppisar
Target Milestone: ---Flags: ppisar: fedora-review+
gwync: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: perl-App-s2p-1.001-1.fc21 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2014-06-26 12:24:07 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

Description Jitka Plesnikova 2014-06-19 14:37:30 UTC
Spec URL: http://jplesnik.fedorapeople.org/perl-App-s2p/perl-App-s2p.spec
SRPM URL: http://jplesnik.fedorapeople.org/perl-App-s2p/perl-App-s2p-1.001-1.fc21.src.rpm
Description: 
This package delivers s2p tool which converts sed scripts to Perl programs.


Fedora Account System Username:jplesnik

---
This package will dual-live with perl.spec's sub-package (after it will be sub-packaged).

Comment 1 Petr Pisar 2014-06-20 09:23:24 UTC
The package provides /usr/bin directory which collides with filesystem package.
I would change the package group to Development/Tools.
The package description has a grammar mistake (tools which converts).
The package does not need the "perl(:MODULE_COMPAT...)" probably as it does not deliver any system-wide module or does not depend on libperl.so.
The package should conflict with perl < 4:5.18.2-300.

The files were sub-packaged from perl.spec in perl-5.18.2-300.fc21.

Comment 2 Jitka Plesnikova 2014-06-24 07:40:07 UTC
Updated. Links are the same.

Comment 3 Petr Pisar 2014-06-24 11:28:03 UTC
URL and Source0 are usable. Ok.
Source archive is original (SHA-256: bf147b7268a46bef07b4046c3f67504a7be482bbf25ccd2abec8abcf2d95375a). Ok.
Summary is Ok.
Description is Ok.
License verified from README, LICENSE, and script/s2p.

FIX: The script/s2p license is:

This program is free and open software. You may use, modify,
distribute, and sell this program (and any modified variants) in any
way you wish, provided you do not restrict others from doing the same.

This does not look like the GPL or the Artistic license. Please negotiate with the upstream a change, or ask Fedora legal department for a new license identifier.

No XS code, noarch BuildArch is Ok.

TODO: Constrain `perl(ExtUtils::MakeMaker)' build-requirement with `>= 6.30' (Makefile.PL:10).

$ rpmlint perl-App-s2p.spec ../SRPMS/perl-App-s2p-1.001-1.fc21.src.rpm ../RPMS/noarch/perl-App-s2p-1.001-1.fc21.noarch.rpm 
perl-App-s2p.src: W: spelling-error Summary(en_US) sed -> tied, ed, seed
perl-App-s2p.src: W: spelling-error %description -l en_US sed -> tied, ed, seed
perl-App-s2p.noarch: W: spelling-error Summary(en_US) sed -> tied, ed, seed
perl-App-s2p.noarch: W: spelling-error %description -l en_US sed -> tied, ed, seed
2 packages and 1 specfiles checked; 0 errors, 4 warnings.
rpmlint is Ok.

$ rpm -q -lv -p ../RPMS/noarch/perl-App-s2p-1.001-1.fc21.noarch.rpm 
-rwxr-xr-x    1 root    root                    53275 Jun 24 13:12 /usr/bin/psed
-rwxr-xr-x    1 root    root                    53275 Jun 24 13:12 /usr/bin/s2p
drwxr-xr-x    2 root    root                        0 Jun 24 13:12 /usr/share/doc/perl-App-s2p
-rw-r--r--    1 root    root                      230 Dec  4  2013 /usr/share/doc/perl-App-s2p/Changes
-rw-r--r--    1 root    root                    18356 Dec  4  2013 /usr/share/doc/perl-App-s2p/LICENSE
-rw-r--r--    1 root    root                      279 Dec  4  2013 /usr/share/doc/perl-App-s2p/README
-rw-r--r--    1 root    root                     8907 Jun 24 13:12 /usr/share/man/man1/psed.1.gz
-rw-r--r--    1 root    root                     8906 Jun 24 13:12 /usr/share/man/man1/s2p.1.gz
File permissions and layout are Ok.

$ rpm -q --requires -p ../RPMS/noarch/perl-App-s2p-1.001-1.fc21.noarch.rpm | sort -f | uniq -c
      1 /usr/bin/perl
      1 perl(Config)
      1 perl(integer)
      1 perl(strict)
      1 perl(Symbol)
      1 perl(vars)
      1 rpmlib(CompressedFileNames) <= 3.0.4-1
      1 rpmlib(FileDigests) <= 4.6.0-1
      1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1
      1 rpmlib(PayloadIsXz) <= 5.2-1
Binary requires are Ok.

$ rpm -q --provides -p ../RPMS/noarch/perl-App-s2p-1.001-1.fc21.noarch.rpm | sort -f | uniq -c
      1 perl-App-s2p = 1.001-1.fc21
Binary provides are Ok.

$ resolvedeps rawhide ../RPMS/noarch/perl-App-s2p-1.001-1.fc21.noarch.rpm 
Binary dependencies resolvable. Ok.

Package builds in F21 (http://koji.fedoraproject.org/koji/taskinfo?taskID=7070741). Ok.

Otherwise the package is in line with Fedora and Perl packaging guidelines.

Please correct all `FIX' items and provide updated package.
Resolution: NOT APPROVED.

Comment 4 Jitka Plesnikova 2014-06-25 15:02:18 UTC
The license has been added as "App-s2p" by Fedora legal. It is Free but GPL-incompatible.

Spec file was updated and links are the same.

Comment 5 Petr Pisar 2014-06-25 15:16:08 UTC
Spec file changes:

--- perl-App-s2p.spec.old       2014-06-24 09:39:36.000000000 +0200
+++ perl-App-s2p.spec   2014-06-25 16:51:01.000000000 +0200
@@ -2,7 +2,7 @@
 Version:        1.001
 Release:        1%{?dist}
 Summary:        Convert sed script to Perl program
-License:        GPL+ or Artistic
+License:        (GPL+ or Artistic) and App-s2p
 Group:          Development/Tools
 URL:            http://search.cpan.org/dist/App-s2p/
 Source0:        http://www.cpan.org/authors/id/L/LE/LEONT/App-s2p-%{version}.tar.gz
@@ -10,7 +10,7 @@
 BuildRequires:  perl
 BuildRequires:  perl(Config)
 BuildRequires:  perl(Devel::FindPerl) >= 0.009
-BuildRequires:  perl(ExtUtils::MakeMaker)
+BuildRequires:  perl(ExtUtils::MakeMaker) >= 6.30
 BuildRequires:  perl(File::Copy)
 BuildRequires:  perl(File::Spec)
 BuildRequires:  perl(integer)

> FIX: The script/s2p license
-License:        GPL+ or Artistic
+License:        (GPL+ or Artistic) and App-s2p
Ok.

> TODO: Constrain `perl(ExtUtils::MakeMaker)' build-requirement with `>= 6.30'
> (Makefile.PL:10).
-BuildRequires:  perl(ExtUtils::MakeMaker)
+BuildRequires:  perl(ExtUtils::MakeMaker) >= 6.30
Ok.

$ rpmlint perl-App-s2p.spec ../SRPMS/perl-App-s2p-1.001-1.fc21.src.rpm ../RPMS/noarch/perl-App-s2p-1.001-1.fc21.noarch.rpm 
perl-App-s2p.src: W: spelling-error Summary(en_US) sed -> tied, ed, seed
perl-App-s2p.src: W: spelling-error %description -l en_US sed -> tied, ed, seed
perl-App-s2p.src: W: invalid-license App-s2p
perl-App-s2p.noarch: W: spelling-error Summary(en_US) sed -> tied, ed, seed
perl-App-s2p.noarch: W: spelling-error %description -l en_US sed -> tied, ed, seed
perl-App-s2p.noarch: W: invalid-license App-s2p
2 packages and 1 specfiles checked; 0 errors, 6 warnings.
rpmlint is Ok.

Resolution: Package APPROVED.

Comment 6 Jitka Plesnikova 2014-06-26 07:27:09 UTC
New Package SCM Request
=======================
Package Name: perl-App-s2p
Short Description: Convert sed script to Perl program
Upstream URL: http://search.cpan.org/dist/App-s2p/
Owners: jplesnik ppisar psabata
Branches:
InitialCC: perl-sig

Comment 7 Gwyn Ciesla 2014-06-26 11:56:43 UTC
Git done (by process-git-requests).

Comment 8 Jitka Plesnikova 2014-06-26 12:24:07 UTC
Thank you for the review and the repository.