Bug 808884 (perl-Data-Clone)

Source file is original. Ok.
URL and Source0 are usable. Ok.
Summary verified from lib/Data/Clone.pm. Ok.
License verified from lib/Data/Clone.pm. Ok.
Description verified from lib/Data/Clone.pm. Ok.
Package compiles XS code, the architecture specific BuildArch is Ok.

All releases provides perl(Test::More) >= 0.88, nonqualified build-require is Ok.
All releases provides perl(Test::Release) >= 0.03, nonqualified build-require is Ok.

FIX: Build-require `perl(inc::Module::Install)' (Makefile.PL:3) instead of `perl(ExtUtils::MakeMaker)'.

TODO: Consider removing bundled Perl modules under inc/ and build-requiring appropriate modules like Module::Install::AuthorTests. Otherwise you need explicitly build-require all dependencies of inc/* code.

FIX: Build-require modules used by code under inc/ (ExtUtils::Manifest, YAML::Tiny, etc.). I really recommend unbundling inc/*.

TODO: Build-require `perl(threads)' for optional tests (t/10_threads.t:4).
TODO: Build-require `perl(constant)' which can dual-live in the future (t/10_threads.t:4).

All tests pass. Ok.

$ rpmlint perl-Data-Clone.spec  ../SRPMS/perl-Data-Clone-0.003-1.fc18.src.rpm  ../RPMS/x86_64/perl-Data-Clone-*
perl-Data-Clone.x86_64: W: devel-file-in-non-devel-package /usr/lib64/perl5/vendor_perl/auto/Data/Clone/data_clone.h
3 packages and 1 specfiles checked; 0 errors, 1 warnings.
rpmlint is Ok.

$ rpm -q -lv -p ../RPMS/x86_64/perl-Data-Clone-0.003-1.fc18.x86_64.rpm
drwxr-xr-x    2 root    root                        0 May  4 13:51 /usr/lib64/perl5/vendor_perl/Data
-rw-r--r--    1 root    root                     5187 Jan 15  2011 /usr/lib64/perl5/vendor_perl/Data/Clone.pm
drwxr-xr-x    2 root    root                        0 May  4 13:51 /usr/lib64/perl5/vendor_perl/auto/Data
drwxr-xr-x    2 root    root                        0 May  4 13:51 /usr/lib64/perl5/vendor_perl/auto/Data/Clone
-rwxr-xr-x    1 root    root                    14688 May  4 13:51 /usr/lib64/perl5/vendor_perl/auto/Data/Clone/Clone.so
-rw-r--r--    1 root    root                      365 Jan 15  2011 /usr/lib64/perl5/vendor_perl/auto/Data/Clone/data_clone.h
drwxr-xr-x    2 root    root                        0 May  4 13:51 /usr/share/doc/perl-Data-Clone-0.003
-rw-r--r--    1 root    root                      291 Jan 15  2011 /usr/share/doc/perl-Data-Clone-0.003/Changes
-rw-r--r--    1 root    root                      505 Jan 15  2011 /usr/share/doc/perl-Data-Clone-0.003/README
-rw-r--r--    1 root    root                     3680 May  4 13:51 /usr/share/man/man3/Data::Clone.3pm.gz
File permissions and layout are Ok.

$ rpm -q --requires -p ../RPMS/x86_64/perl-Data-Clone-0.003-1.fc18.x86_64.rpm |sort |uniq -c
      1 libc.so.6()(64bit)
      1 libc.so.6(GLIBC_2.11)(64bit)
      1 libc.so.6(GLIBC_2.2.5)(64bit)
      1 perl >= 0:5.008_001
      2 perl(Exporter)
      1 perl(:MODULE_COMPAT_5.14.2)
      1 perl(parent)
      1 perl(strict)
      1 perl(XSLoader)
      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
      1 rtld(GNU_HASH)
TODO: Do not explicitly run-require auto-discovered perl(Exporter).

$ rpm -q --provides  -p ../RPMS/x86_64/perl-Data-Clone-0.003-1.fc18.x86_64.rpm |sort |uniq -c
      1 perl(Data::Clone) = 0.003
      1 perl-Data-Clone = 0.003-1.fc18
      1 perl-Data-Clone(x86-64) = 0.003-1.fc18
Binary provides are Ok.

$ resolvedeps rawhide  ../RPMS/x86_64/perl-Data-Clone-0.003-1.fc18.x86_64.rpm 
Binary dependencies resolvable. Ok.

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

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


Please correct all `FIX' prefixed issues, consider fixing `TODO' items, and provide new spec file.

Resolution: Package NOT approved.

(In reply to comment #1)
> 
> FIX: Build-require `perl(inc::Module::Install)' (Makefile.PL:3) instead of
> `perl(ExtUtils::MakeMaker)'.
> 
> TODO: Consider removing bundled Perl modules under inc/ and build-requiring
> appropriate modules like Module::Install::AuthorTests. Otherwise you need
> explicitly build-require all dependencies of inc/* code.
> 
> FIX: Build-require modules used by code under inc/ (ExtUtils::Manifest,
> YAML::Tiny, etc.). I really recommend unbundling inc/*.

Oh! This is new. I'm not convinced that avoiding bundled Module::Install is the way to go here. The packaged inc::Module::Install even states
# This module ONLY loads if the user has manually installed their own
# installation of Module::Install, and are some form of MI author.

The additional dependencies under inc/ are either optional (e.g. YAML::Tiny for writing MYMETA.yml which we don't need) or already pulled in by EU::MM. Since Module::Install is intended to work with nothing more than core perl installed, I don't see any reason to explicitly mention dependencies unless there's an unbundled core module missing that causes build failure.

Iain,

minimal build root does not guarantee perl-core being available. It's not acceptable to rely on transitive dependencies (Perl dependencies are tracked by modules, not RPM packages in Fedora). If your code needs a dependency directly, it's has to be declared in the spec file.

If you think inc/* files are not needed because they are already available from system (be ware Module::Install::AuthorTests is not part of EU::MM), then why do you restrain from removing them?

(In reply to comment #2)
> (In reply to comment #1)
> > 
> > FIX: Build-require `perl(inc::Module::Install)' (Makefile.PL:3) instead of
> > `perl(ExtUtils::MakeMaker)'.
> > 
> > TODO: Consider removing bundled Perl modules under inc/ and build-requiring
> > appropriate modules like Module::Install::AuthorTests. Otherwise you need
> > explicitly build-require all dependencies of inc/* code.
> > 
> > FIX: Build-require modules used by code under inc/ (ExtUtils::Manifest,
> > YAML::Tiny, etc.). I really recommend unbundling inc/*.
> 
> Oh! This is new.
IMO, bundled modules in inc are harmless as long as they do not contain modules which later on are being used as run time deps.

> I'm not convinced that avoiding bundled Module::Install is
> the way to go here.
IMO, Module::Install are just "build-/install instruction file fragments" and therefore are OK to be used and to be bundled.

Yes, it's possible to keep the inc files there. Even sometimes it's necessary because upstream bundles incompatible or modified version.

I only think it's easier for packager to unbundle them and build-require two or three lines, than reviewing them to extract all their dependencies (or worrying when build fails because sub-packaging some core module).

Updated to BR perl(inc::Module::Install) instead of EU::MM.

Spec URL: http://fedorapeople.org/~iarnell/review/perl-Data-Clone.spec
SRPM URL: http://fedorapeople.org/~iarnell/review/perl-Data-Clone-0.003-2.fc18.src.rpm

Koji URL: https://koji.fedoraproject.org/koji/taskinfo?taskID=4121574

Spec file changes:
--- perl-Data-Clone.spec.old    2012-04-01 15:45:49.000000000 +0200
+++ perl-Data-Clone.spec        2012-06-02 16:13:07.000000000 +0200
@@ -1,21 +1,23 @@
 Name:           perl-Data-Clone
 Version:        0.003
-Release:        1%{?dist}
+Release:        2%{?dist}
 Summary:        Polymorphic data cloning
 License:        GPL+ or Artistic
 Group:          Development/Libraries
 URL:            http://search.cpan.org/dist/Data-Clone/
 Source0:        http://www.cpan.org/authors/id/G/GF/GFUJI/Data-Clone-%{version}.tar.gz
-BuildRequires:  perl >= 1:5.8.1
+BuildRequires:  perl(constant)
 BuildRequires:  perl(Data::Dumper)
 BuildRequires:  perl(Devel::PPPort) >= 3.19
-BuildRequires:  perl(ExtUtils::MakeMaker)
 BuildRequires:  perl(ExtUtils::ParseXS) >= 2.21
+BuildRequires:  perl(inc::Module::Install)
+BuildRequires:  perl(Module::Install::AuthorTests)
 BuildRequires:  perl(parent)
 BuildRequires:  perl(Scalar::Util)
 BuildRequires:  perl(Test::LeakTrace)
 BuildRequires:  perl(Test::More)
 BuildRequires:  perl(Test::Requires)
+BuildRequires:  perl(threads)
 BuildRequires:  perl(Tie::Array)
 BuildRequires:  perl(Tie::Hash)
 BuildRequires:  perl(Time::HiRes)
@@ -59,5 +61,9 @@
 %{_mandir}/man3/*

 %changelog
+* Fri Jun 01 2012 Iain Arnell <iarnell> 0.003-2
+- BuildRequire inc::Module::Install, not EU::MM
+- additional BuildRequires for tests
+
 * Sun Apr 01 2012 Iain Arnell <iarnell> 0.003-1
 - Specfile autogenerated by cpanspec 1.79.

> FIX: Build-require `perl(inc::Module::Install)' (Makefile.PL:3) instead of
> `perl(ExtUtils::MakeMaker)'.
-BuildRequires:  perl(ExtUtils::MakeMaker)
 BuildRequires:  perl(ExtUtils::ParseXS) >= 2.21
+BuildRequires:  perl(inc::Module::Install)
+BuildRequires:  perl(Module::Install::AuthorTests)
Ok.

> TODO: Consider removing bundled Perl modules under inc/ and build-requiring
> appropriate modules like Module::Install::AuthorTests. Otherwise you need explicitly
> build-require all dependencies of inc/* code.
> FIX: Build-require modules used by code under inc/ (ExtUtils::Manifest, YAML::Tiny,
> etc.). I really recommend unbundling inc/*.
Well, you build-require the inc::Module::Install and Module::Install::AuthorTests now, but because you left the bundled files in inc, they are still used instead of the system ones:

# strace -eopen perl Makefile.PL 2>&1 |grep Install
open("/usr/share/perl5/vendor_perl/inc/Module/Install.pm", O_RDONLY) = 4
open("inc/Module/Install.pm", O_RDONLY) = 4
open("inc/Module/Install/XSUtil.pm", O_RDONLY) = 4
open("inc/Module/Install/Base.pm", O_RDONLY) = 5
open("inc/Module/Install/AuthorTests.pm", O_RDONLY) = 4
open("inc/Module/Install/Repository.pm", O_RDONLY) = 4
open("inc/Module/Install/WriteAll.pm", O_RDONLY) = 4
open("inc/Module/Install/Metadata.pm", O_RDONLY) = 4
open("inc/Module/Install/Can.pm", O_RDONLY) = 4
open("inc/Module/Install/Makefile.pm", O_RDONLY) = 4

TODO: That means you still need to build-require all modules used from those files. I believe you can see the reasoning for removing the inc directory now and you will do it.

> TODO: Build-require `perl(threads)' for optional tests (t/10_threads.t:4).
+BuildRequires:  perl(threads)
Ok.

> TODO: Build-require `perl(constant)' which can dual-live in the future
> (t/10_threads.t:4).
+BuildRequires:  perl(constant)
Ok.

All tests pass. Ok.

> TODO: Do not explicitly run-require auto-discovered perl(Exporter).
$ rpm -q --requires -p ../RPMS/x86_64/perl-Data-Clone-0.003-2.fc18.x86_64.rpm |sort |uniq -c
      1 libc.so.6()(64bit)
      1 libc.so.6(GLIBC_2.11)(64bit)
      1 libc.so.6(GLIBC_2.2.5)(64bit)
      1 perl >= 0:5.008_001
      2 perl(Exporter)
      1 perl(:MODULE_COMPAT_5.14.2)
      1 perl(parent)
      1 perl(strict)
      1 perl(XSLoader)
      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
      1 rtld(GNU_HASH)
Not addressed. Ok.

Package build is F18 (http://koji.fedoraproject.org/koji/taskinfo?taskID=4124880). Ok.


Please consider purging the inc directory before building this package.

Resolution: Package APPROVED.

New Package SCM Request
=======================
Package Name: perl-Data-Clone
Short Description: Polymorphic data cloning
Owners: iarnell
Branches: f16 f17
InitialCC: perl-sig

Git done (by process-git-requests).

perl-Data-Clone-0.003-2.fc16.1 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/perl-Data-Clone-0.003-2.fc16.1

perl-Data-Clone-0.003-2.fc17.1 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/perl-Data-Clone-0.003-2.fc17.1

perl-Data-Clone-0.003-2.fc16.1 has been pushed to the Fedora 16 testing repository.

perl-Data-Clone-0.003-2.fc16.1 has been pushed to the Fedora 16 stable repository.

perl-Data-Clone-0.003-2.fc17.1 has been pushed to the Fedora 17 stable repository.