Bug 1120293 - Review Request: perl-Inline-C - Write Perl subroutines in C
Summary: Review Request: perl-Inline-C - Write Perl subroutines in C
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: 1118648 1120297 1120299
TreeView+ depends on / blocked
 
Reported: 2014-07-16 16:29 UTC by Petr Šabata
Modified: 2014-08-01 12:22 UTC (History)
2 users (show)

Fixed In Version: perl-Inline-C-0.60-1.fc21
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2014-08-01 12:22:13 UTC
ppisar: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Petr Šabata 2014-07-16 16:29:42 UTC
Spec URL: http://psabata.fedorapeople.org/pkgs/perl-Inline-C/perl-Inline-C.spec
SRPM URL: http://psabata.fedorapeople.org/pkgs/perl-Inline-C/perl-Inline-C-0.60-1.fc20.src.rpm
Description:
Inline::C is a module that allows you to write Perl subroutines in C. Since
version 0.30 the Inline module supports multiple programming languages and
each language has its own support module.
Fedora Account System Username: psabata

Comment 1 Petr Pisar 2014-07-17 06:39:42 UTC
URL and Source0 are usable. Ok.
Source tar ball is original (SHA-256: f8593750d45d1c068195b4f16643fe4b3d50956e79d26c3c8aa7ba887ae75a45). Ok.
Summary verified from README. Ok.
Description verified from README. Ok.

FATAL: lib/Inline/C/Cookbook.pod license is Artistic:

=head1 COPYRIGHT

Copyright 2000-2014. Ingy döt Net.

Copyright 2008, 2010-2014. Sisyphus.

See L<http://www.perl.com/perl/misc/Artistic.html>

The URL points to The "Artistic License" license which is not allowed by Fedora. Please remove the file from source archive, or clarify the license with the upstream and Fedora legal department.

Comment 2 Petr Šabata 2014-07-17 08:13:08 UTC
I removed the offending file from the distribution for now.

Comment 3 Petr Pisar 2014-07-17 08:41:40 UTC
You need to remove the file from Inline-C-0.60.tar.gz archive. Deleting in the SPEC file is too late.

Comment 4 Petr Šabata 2014-07-17 13:15:22 UTC
Alright, removed.  Links updated.

Comment 5 Petr Pisar 2014-07-17 14:10:24 UTC
URL and Source0 are usable. Ok.
Source tar ball is modified (original SHA-256: f8593750d45d1c068195b4f16643fe4b3d50956e79d26c3c8aa7ba887ae75a45). lib/Inline/C/Cookbook.pod has been removed and MANIFEST updated. Ok.
Summary verified from README. Ok.
Description verified from README. Ok.
License verified from lib/Inline/C.pod, lib/Inline/C/ParseRecDescent.pod, lib/Inline/C/ParseRegExp.pod, LICENSE, README. Ok.
No XS is code in the binary packages, noarch BuildArch is Ok.

TODO: Due to NO_PACKLIST=1 added in recent ExtUtil::MakeMaker, you should increase perl(ExtUtils::MakeMaker) build-time dependency to `>= 6.76'.

The Concflicts is Ok.

TODO: Constrain `perl(File::Spec)' dependency with `>= 0.8' (Makefile.PL:23).
TODO: Constrain `perl(Parse::RecDescent)' dependency with `>= 1.80' (Makefile.PL:25).
TODO: Constrain `perl(Test::More)' dependency with `>= 0.88' (Makefile.PL:45).
TODO: Constrain `perl(Test::Warn)' dependency with `>= 0.23' (Makefile.PL:46).

TODO: I recommend to conditionalize dependencies on Inline::Filters and Inline::Struct by %perl_bootstrap macro as they are optional as Parse::RecDescent.

Test::Pod is not used. Ok.

All tests pass. Ok.

$ rpmlint perl-Inline-C.spec ../SRPMS/perl-Inline-C-0.60-1.fc22.src.rpm ../RPMS/noarch/perl-Inline-C-0.60-1.fc22.noarch.rpm 
perl-Inline-C.spec:8: W: macro-in-comment %{version}
perl-Inline-C.spec:9: W: macro-in-comment %{version}
perl-Inline-C.spec:10: W: macro-in-comment %{version}
perl-Inline-C.spec:11: W: macro-in-comment %{version}
perl-Inline-C.spec:12: W: macro-in-comment %{version}
perl-Inline-C.spec:12: W: macro-in-comment %{version}
perl-Inline-C.spec: W: invalid-url Source0: Inline-C-0.60.fedora.tar.gz
perl-Inline-C.src:8: W: macro-in-comment %{version}
perl-Inline-C.src:9: W: macro-in-comment %{version}
perl-Inline-C.src:10: W: macro-in-comment %{version}
perl-Inline-C.src:11: W: macro-in-comment %{version}
perl-Inline-C.src:12: W: macro-in-comment %{version}
perl-Inline-C.src:12: W: macro-in-comment %{version}
perl-Inline-C.src: W: invalid-url Source0: Inline-C-0.60.fedora.tar.gz
2 packages and 1 specfiles checked; 0 errors, 14 warnings.
TODO: Escape % in comments.

$ rpm -q -lv -p ../RPMS/noarch/perl-Inline-C-0.60-1.fc22.noarch.rpm 
drwxr-xr-x    2 root    root                        0 Jul 17 15:50 /usr/share/doc/perl-Inline-C
-rw-r--r--    1 root    root                     1267 Jul 14 07:04 /usr/share/doc/perl-Inline-C/CONTRIBUTING
-rw-r--r--    1 root    root                     5435 Jul 14 07:04 /usr/share/doc/perl-Inline-C/Changes
-rw-r--r--    1 root    root                    18350 Jul 14 07:04 /usr/share/doc/perl-Inline-C/LICENSE
-rw-r--r--    1 root    root                    19756 Jul 14 07:04 /usr/share/doc/perl-Inline-C/README
-rw-r--r--    1 root    root                     9345 Jul 17 15:48 /usr/share/man/man3/Inline::C.3pm.gz
-rw-r--r--    1 root    root                     2159 Jul 17 15:48 /usr/share/man/man3/Inline::C::ParseRecDescent.3pm.gz
-rw-r--r--    1 root    root                     2159 Jul 17 15:48 /usr/share/man/man3/Inline::C::ParseRegExp.3pm.gz
drwxr-xr-x    2 root    root                        0 Jul 17 15:48 /usr/share/perl5/vendor_perl/Inline/C
-rw-r--r--    1 root    root                    33308 Jul 14 07:04 /usr/share/perl5/vendor_perl/Inline/C.pm
-rw-r--r--    1 root    root                    19197 Jul 14 07:04 /usr/share/perl5/vendor_perl/Inline/C.pod
-rw-r--r--    1 root    root                     4122 Jul 14 07:04 /usr/share/perl5/vendor_perl/Inline/C/ParseRecDescent.pm
-rw-r--r--    1 root    root                      775 Jul 14 07:04 /usr/share/perl5/vendor_perl/Inline/C/ParseRecDescent.pod
-rw-r--r--    1 root    root                     5576 Jul 14 07:04 /usr/share/perl5/vendor_perl/Inline/C/ParseRegExp.pm
-rw-r--r--    1 root    root                      743 Jul 14 07:04 /usr/share/perl5/vendor_perl/Inline/C/ParseRegExp.pod
File layout and permissions are Ok.

$ rpm -q --requires -p ../RPMS/noarch/perl-Inline-C-0.60-1.fc22.noarch.rpm | sort -f | uniq -c
      1 perl(:MODULE_COMPAT_5.18.2)
      1 perl(Carp)
      1 perl(Config)
      1 perl(Cwd)
      1 perl(Data::Dumper)
      1 perl(ExtUtils::MakeMaker)
      1 perl(Fcntl)
      1 perl(File::Spec)
      1 perl(Inline) >= 0.56
      1 perl(Inline) >= 0.58
      1 perl(strict)
      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
TODO: Remove under-specified dependency on `perl(Inline) >= 0.56'.
FIX: Run-require `perl(FindBin)' (lib/Inline/C.pm:47).
TODO: I recommend to run-require Inline::Filters, Inline::Struct, Parse::RecDescent, Time::HiRes. The code croaks without them. This is just a prettier die.

$ rpm -q --provides -p ../RPMS/noarch/perl-Inline-C-0.60-1.fc22.noarch.rpm | sort -f | uniq -c
      1 perl(Inline::C) = 0.60
      1 perl(Inline::C::ParseRecDescent)
      1 perl(Inline::C::ParseRegExp)
      1 perl-Inline-C = 0.60-1.fc22
Binary provides are Ok.

resolvedeps f22-build ../RPMS/noarch/perl-Inline-C-0.60-1.fc22.noarch.rpm
Binary dependencies resolvable. Ok.

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

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

Please correct all `FIX' items, consider fixing `TODO' items, and provide new spec file.
Resolution: Package NOT approved.

Comment 6 Petr Šabata 2014-07-17 14:24:46 UTC
(In reply to Petr Pisar from comment #5)
> TODO: I recommend to conditionalize dependencies on Inline::Filters and
> Inline::Struct by %perl_bootstrap macro as they are optional as
> Parse::RecDescent.

That's planned once they get into Fedora.

> TODO: Due to NO_PACKLIST=1 added in recent ExtUtil::MakeMaker, you should
> increase perl(ExtUtils::MakeMaker) build-time dependency to `>= 6.76'.

Done.

> TODO: Escape % in comments.

Done.

> FIX: Run-require `perl(FindBin)' (lib/Inline/C.pm:47).

Done.

> TODO: I recommend to run-require Inline::Filters, Inline::Struct,
> Parse::RecDescent, Time::HiRes. The code croaks without them. This is just a
> prettier die.

Planned.  Will be done once they're available.

Links updated.

Comment 7 Petr Pisar 2014-07-17 14:51:55 UTC
Spec file changes:

$ diff -u perl-Inline-C.spec{.old,}
--- perl-Inline-C.spec.old      2014-07-17 15:13:52.000000000 +0200
+++ perl-Inline-C.spec  2014-07-17 16:24:15.000000000 +0200
@@ -6,10 +6,10 @@
 Group:          Development/Libraries
 URL:            http://search.cpan.org/dist/Inline-C/
 # Created from http://www.cpan.org/authors/id/I/IN/INGY/Inline-C-%{version}.tar.gz
-# tar xzf Inline-C-%{version}.tar.gz && \
-#   rm -f Inline-C-%{version}/lib/Inline/C/Cookbook.pod && \
-#   sed -e '/Cookbook.pod$/d' -i Inline-C-%{version}/MANIFEST && \
-#   tar czf Inline-C-%{version}.fedora.tar.gz Inline-C-%{version}
+# tar xzf Inline-C-%%{version}.tar.gz && \
+#   rm -f Inline-C-%%{version}/lib/Inline/C/Cookbook.pod && \
+#   sed -e '/Cookbook.pod$/d' -i Inline-C-%%{version}/MANIFEST && \
+#   tar czf Inline-C-%%{version}.fedora.tar.gz Inline-C-%{version}
 Source0:        Inline-C-%{version}.fedora.tar.gz
 BuildArch:      noarch
 BuildRequires:  perl
@@ -19,7 +19,7 @@
 BuildRequires:  perl(Cwd)
 BuildRequires:  perl(Data::Dumper)
 BuildRequires:  perl(diagnostics)
-BuildRequires:  perl(ExtUtils::MakeMaker) >= 6.30
+BuildRequires:  perl(ExtUtils::MakeMaker) >= 6.76
 BuildRequires:  perl(Fcntl)
 BuildRequires:  perl(File::Path)
 BuildRequires:  perl(File::Spec)
@@ -34,6 +34,7 @@
 BuildRequires:  perl(Test::Warn)
 BuildRequires:  perl(Time::HiRes)
 BuildRequires:  perl(warnings)
+Requires:       perl(FindBin)
 Requires:       perl(Inline) >= 0.58
 Requires:       perl(:MODULE_COMPAT_%(eval "$(perl -V:version)"; echo $version))
 # Split from Inline in 0.58


> TODO: Due to NO_PACKLIST=1 added in recent ExtUtil::MakeMaker, you should
> increase perl(ExtUtils::MakeMaker) build-time dependency to `>= 6.76'.
-BuildRequires:  perl(ExtUtils::MakeMaker) >= 6.30
+BuildRequires:  perl(ExtUtils::MakeMaker) >= 6.76
Ok.

> TODO: Constrain `perl(File::Spec)' dependency with `>= 0.8' (Makefile.PL:23).
Not addressed.

> TODO: Constrain `perl(Parse::RecDescent)' dependency with `>= 1.80'
> (Makefile.PL:25).
Not addressed.

> TODO: Constrain `perl(Test::More)' dependency with `>= 0.88' (Makefile.PL:45).
Not addressed.

> TODO: Constrain `perl(Test::Warn)' dependency with `>= 0.23' (Makefile.PL:46).
Not addressed.

> TODO: I recommend to conditionalize dependencies on Inline::Filters and
> Inline::Struct by %perl_bootstrap macro as they are optional as
> Parse::RecDescent.
Not addressed. One can add %global perl_bootstrap 1 at the beginning of the spec file now and use the macros now.

$ rpmlint  perl-Inline-C.spec ../SRPMS/perl-Inline-C-0.60-1.fc22.src.rpm ../RPMS/noarch/perl-Inline-C-0.60-1.fc22.noarch.rpm 
perl-Inline-C.spec:8: W: macro-in-comment %{version}
perl-Inline-C.spec:12: W: macro-in-comment %{version}
perl-Inline-C.spec: W: invalid-url Source0: Inline-C-0.60.fedora.tar.gz
perl-Inline-C.src:8: W: macro-in-comment %{version}
perl-Inline-C.src:12: W: macro-in-comment %{version}
perl-Inline-C.src: W: invalid-url Source0: Inline-C-0.60.fedora.tar.gz
2 packages and 1 specfiles checked; 0 errors, 6 warnings.

> TODO: Remove under-specified dependency on `perl(Inline) >= 0.56'.
Not addressed.

$ rpm -q --requires -p  ../RPMS/noarch/perl-Inline-C-0.60-1.fc22.noarch.rpm | sort -f | uniq -c
      1 perl(:MODULE_COMPAT_5.18.2)
      1 perl(Carp)
      1 perl(Config)
      1 perl(Cwd)
      1 perl(Data::Dumper)
      1 perl(ExtUtils::MakeMaker)
      1 perl(Fcntl)
      1 perl(File::Spec)
      1 perl(FindBin)
      1 perl(Inline) >= 0.56
      1 perl(Inline) >= 0.58
      1 perl(strict)
      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

> TODO: Remove under-specified dependency on `perl(Inline) >= 0.56'.
Not addressed.

> FIX: Run-require `perl(FindBin)' (lib/Inline/C.pm:47).
Ok.

> TODO: I recommend to run-require Inline::Filters, Inline::Struct,
> Parse::RecDescent, Time::HiRes. The code croaks without them. This is just a
> prettier die.
Not addressed.

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

Please consider fixing the `TODO' items before building the package.
Resolution: Package APPROVED.

Comment 8 Petr Šabata 2014-07-17 15:07:53 UTC
New Package SCM Request
=======================
Package Name: perl-Inline-C
Short Description: Write Perl subroutines in C
Upstream URL: http://search.cpan.org/dist/Inline-C/
Owners: psabata jplesnik ppisar
Branches: f21
InitialCC: perl-sig

Comment 9 Gwyn Ciesla 2014-07-17 15:47:09 UTC
Git done (by process-git-requests).

Comment 10 Petr Šabata 2014-07-17 15:54:50 UTC
Thank you, both.


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