Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 1230209 - Review Request: perl-Test-Time - Overrides the time() and sleep() core functions for testing
Summary: Review Request: perl-Test-Time - Overrides the time() and sleep() core functi...
Keywords:
Status: CLOSED DEFERRED
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: AwaitingSubmitter
Depends On:
Blocks: FE-DEADREVIEW 1230213
TreeView+ depends on / blocked
 
Reported: 2015-06-10 12:40 UTC by Ralf Corsepius
Modified: 2015-11-25 15:17 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-11-25 15:17:17 UTC
ppisar: fedora-review-


Attachments (Terms of Use)


Links
System ID Priority Status Summary Last Updated
Red Hat Bugzilla 1033481 None None None Never

Internal Links: 1033481

Description Ralf Corsepius 2015-06-10 12:40:04 UTC
Spec URL: https://corsepiu.fedorapeople.org/packages/perl-Test-Time.spec
SRPM URL: https://corsepiu.fedorapeople.org/packages/perl-Test-Time-0.04-1.fc23.src.rpm

Description:
Test::Time can be used to test modules that deal with time. Once you use
this module, all references to time and sleep will be internalized. You can
set custom time by passing time => number after the use statement.

Fedora Account System Username: corsepiu

Comment 1 Petr Pisar 2015-06-19 10:07:23 UTC
URL and Source0 are usable. Ok.
Source archive is original (SHA-256: d8c1bc57f9767ae8122fc4ab873bd991cb9ea8e9422c66399acb66770fa5c2ea). Ok.
Summary verified from lib/Test/Time.pm. Ok.
Description verified from lib/Test/Time.pm. Ok.
License verified from README and lib/Test/Time.pm. Ok.
No XS code, noarch BuildArch is Ok.

TODO: Use plain `perl' command instead of %{__perl} macro and build-require `perl'.

FIX: Remove all the bundled Module::Install files from ./inc, build-require `perl(inc::Module::Install)' and other needed modules from Module::Install namespace (locate functions called from Makefile.PL). Or declare all build-time dependencies for the bundled Module::Install modules.

FIX: Build-require `coreutils' (perl-Test-Time.spec:32).
FIX: Build-require `sed' (perl-Test-Time.spec:33).
FIX: Build-require `make' (perl-Test-Time.spec:38).
FIX: Build-require `findutils' (perl-Test-Time.spec:43).

FIX: Build-require `perl(strict)' (lib/Test/Time.pm:2).
FIX: Build-require `perl(warnings)' (lib/Test/Time.pm:3).

TODO: Replace PERL_INSTALL_ROOT with DESTDIR argument in %install section.
TODO: Remove the unnecessary find command deleting empty directories from %install section.

All tests pass. Ok.

$ rpmlint perl-Test-Time.spec ../SRPMS/perl-Test-Time-0.04-1.fc23.src.rpm ../RPMS/noarch/perl-Test-Time-0.04-1.fc23.noarch.rpm 
2 packages and 1 specfiles checked; 0 errors, 0 warnings.
rpmlint is Ok.

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

$ rpm -q -lv -p ../RPMS/noarch/perl-Test-Time-0.04-1.fc23.noarch.rpm 
drwxr-xr-x    2 root    root                        0 Jun 19 11:16 /usr/share/doc/perl-Test-Time
-rw-r--r--    1 root    root                      253 Jun 14  2012 /usr/share/doc/perl-Test-Time/Changes
-rw-r--r--    1 root    root                      943 Jun 14  2012 /usr/share/doc/perl-Test-Time/README
-rw-r--r--    1 root    root                     1575 Jun 19 11:16 /usr/share/man/man3/Test::Time.3pm.gz
drwxr-xr-x    2 root    root                        0 Jun 19 11:16 /usr/share/perl5/vendor_perl/Test
-rw-r--r--    1 root    root                     1537 Jun 14  2012 /usr/share/perl5/vendor_perl/Test/Time.pm
File layout and permissions are Ok.

$ rpm -q --requires -p ../RPMS/noarch/perl-Test-Time-0.04-1.fc23.noarch.rpm | sort -f | uniq -c
      1 perl(:MODULE_COMPAT_5.22.0)
      1 perl(strict)
      1 perl(Test::More)
      1 perl(warnings)
      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-Test-Time-0.04-1.fc23.noarch.rpm | sort -f | uniq -c
      1 perl(Test::Time) = 0.04
      1 perl-Test-Time = 0.04-1.fc23
Binary provides are Ok.

$ resolvedeps rawhide ../RPMS/noarch/perl-Test-Time-0.04-1.fc23.noarch.rpm 
Binary dependencies resolvable. 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 3 Petr Pisar 2015-06-26 13:25:27 UTC
Spec file changes:

--- perl-Test-Time.spec.old     2015-06-10 14:36:39.000000000 +0200
+++ perl-Test-Time.spec 2015-06-26 12:06:10.000000000 +0200
@@ -1,11 +1,18 @@
 Name:           perl-Test-Time
 Version:        0.04
-Release:        1%{?dist}
+Release:        2%{?dist}
 Summary:        Overrides the time() and sleep() core functions for testing
 License:        GPL+ or Artistic
 URL:            http://search.cpan.org/dist/Test-Time/
 Source0:        http://www.cpan.org/authors/id/S/SA/SATOH/Test-Time-%{version}.tar.gz
 BuildArch:      noarch
+
+BuildRequires:  perl(inc::Module::Install)
+BuildRequires:  perl(Module::Install::AuthorTests)
+BuildRequires:  perl(Module::Install::Repository)
+BuildRequires:  perl(Module::Install::ReadmeFromPod)
+BuildRequires:  perl(Module::Install::TestBase)
+
 BuildRequires:  perl(ExtUtils::MakeMaker)
 BuildRequires:  perl(Filter::Util::Call)
 BuildRequires:  perl(Test::More)
@@ -21,7 +28,8 @@
 %setup -q -n Test-Time-%{version}
 
 # Remove bundled modules
-for f in inc/Spiffy.pm \
+for f in $(find inc/Module -name *.pm) \
+    inc/Spiffy.pm \
     inc/Test/Base/Filter.pm \
     inc/Test/Name/FromLine.pm \
     inc/Test/More.pm \
@@ -54,5 +62,8 @@
 %{_mandir}/man3/*
 
 %changelog
+* Fri Jun 26 2015 Ralf Corsépius <corsepiu@fedoraproject.org> - 0.04-2
+- Remove bundled inc/Module.
+
 * Mon Jun 08 2015 Ralf Corsépius <corsepiu@fedoraproject.org> - 0.04-1
 - Initial package.


> TODO: Use plain `perl' command instead of %{__perl} macro and build-require
> `perl'.
Not addressed. Ok.

> FIX: Remove all the bundled Module::Install files from ./inc, build-require
> `perl(inc::Module::Install)' and other needed modules from Module::Install
> namespace (locate functions called from Makefile.PL). Or declare all
> build-time dependencies for the bundled Module::Install modules.
+BuildRequires:  perl(inc::Module::Install)
+BuildRequires:  perl(Module::Install::AuthorTests)
+BuildRequires:  perl(Module::Install::Repository)
+BuildRequires:  perl(Module::Install::ReadmeFromPod)
+BuildRequires:  perl(Module::Install::TestBase)

That's fine, but not enough. Even upstream don't know what dependencies needs. Please also:

FIX: Build-require `perl(Module::Install::Metadata)' (Makefile.PL:24).
FIX: BUild-require `perl(Module::Install::Include)' (Makefile.PL:43).


> FIX: Build-require `coreutils' (perl-Test-Time.spec:32).
> FIX: Build-require `sed' (perl-Test-Time.spec:33).
> FIX: Build-require `make' (perl-Test-Time.spec:38).
> FIX: Build-require `findutils' (perl-Test-Time.spec:43).
> FIX: Build-require `perl(strict)' (lib/Test/Time.pm:2).
> FIX: Build-require `perl(warnings)' (lib/Test/Time.pm:3).
FIX: Please add these dependencies.

> TODO: Replace PERL_INSTALL_ROOT with DESTDIR argument in %install section.
Not addressed. Ok.

> TODO: Remove the unnecessary find command deleting empty directories from
> %install section.
Not addresses. Ok.

Please correct all `FIX' items and provide new spec file.

Resolution: Package NOT approved.

Comment 4 Petr Pisar 2015-10-13 08:10:32 UTC
Any progress?

Comment 5 Petr Pisar 2015-11-25 15:17:17 UTC
No progress. Marking this review as dead.


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