Bug 1111242 - Review Request: perl-App-s2p - Convert sed script to Perl program
Summary: Review Request: perl-App-s2p - Convert sed script to Perl program
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Petr Pisar
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2014-06-19 14:37 UTC by Jitka Plesnikova
Modified: 2014-06-26 12:24 UTC (History)
2 users (show)

Fixed In Version: perl-App-s2p-1.001-1.fc21
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2014-06-26 12:24:07 UTC
ppisar: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

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.


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