Bug 1658851 - Review Request: perl-Schedule-Cron - Provides a simple but complete cron like scheduler
Summary: Review Request: perl-Schedule-Cron - Provides a simple but complete cron like...
Keywords:
Status: CLOSED ERRATA
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: 2018-12-13 02:34 UTC by Andrew Bauer
Modified: 2019-01-03 17:44 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2018-12-27 01:50:51 UTC
Type: ---
Embargoed:
ppisar: fedora-review+


Attachments (Terms of Use)

Description Andrew Bauer 2018-12-13 02:34:11 UTC
Not to be confused with perl-Schedule-Cron-Events

Fedora Account System Username: kni

Spec URL: https://raw.githubusercontent.com/knight-of-ni/specfiles/master/perl-Schedule-Cron.spec

SRPM URL: https://kojipkgs.fedoraproject.org//work/tasks/4378/31434378/perl-Schedule-Cron-1.01-1.fc30.src.rpm

Description
------------
This module provides  a simple but complete cron  like scheduler.  I.e
this modules can be  used for periodically executing Perl subroutines.
The  dates  and  parameters  for  the subroutines  to  be  called  are
specified with a format known as crontab entry (see man page crontab(5)
or documentation of Schedule::Cron).

The   philosophy  behind   Schedule::Cron  is   to   call  subroutines
periodically from  within one single  Perl program instead  of letting
cron  trigger several  (possibly different)  Perl  scripts. Everything
under  one  roof.  Furthermore  Schedule::Cron  provides mechanism  to
create crontab entries dynamically, which isn't that easy with cron.

Schedule::Cron  knows  about  all   extensions  (well,  at  least  all
extensions I'm aware of, i.e those  of the so called "Vixie" cron) for
crontab entries like ranges  including 'steps', specification of month
and days of the week by name or coexistence of lists and ranges in the
same field. And  even a bit more (like lists  and ranges with symbolic
names).

This module is rather effective concerning system load.  It calculates
the execution  dates in advance and  will sleep until  those dates are
reached (and  wont wake  up every minute  to check for  execution like
cron).   However, it  relies on  the accuracy  of your  sleep() system
call.

RPMLint
-------
perl-Schedule-Cron.noarch: W: spelling-error %description -l en_US crontab -> contactable
perl-Schedule-Cron.src: W: spelling-error %description -l en_US crontab -> contactable
2 packages and 0 specfiles checked; 0 errors, 2 warnings.

Comment 2 Petr Pisar 2018-12-14 13:57:59 UTC
URL and Source links are Ok.
Source0 (SHA-256: 8212766652e098e23e1ef193b9739e9cc0ed106425cb9f8a50111877cfe02940) is original. Ok.
Source1 (SHA-256: 1daafdb99cd9da730d27e66a4773e8e98d3a431a6fd3ec0ab39a056e0677bf0f) is original. Ok.
Summary is Ok.
Description is Ok.
License verified from lib/Schedule/Cron.pm, examples/custom_sleep.pl, README, and Source1. Ok.

TODO: Remove the obsolete `Group:' tag.

No XS code, noarch BuildArch is Ok.

FIX: Build-require `coreutils' (perl-Schedule-Cron.spec:57).
TODO: Do not build-require `perl-devel`, it's not needed.

TODO: Constrain `perl(ExtUtils::MakeMaker)' build-dependency with `>= 6.76' because of NO_PACKLIST=1 and remove `find %{buildroot} -type f -name .packlist -delete' line from the %install section. Or remove the `NO_PACKLIST=1' argument.

FIX: Remove `find %{buildroot} -type d -empty -delete' line from the %install section. ExtUtils::MakeMaker does not create empty directories, the command is not necessary.

FIX: Constrain `perl(Time::ParseDate)' build-dependency with `>= 2011.0505' (Makefile.PL:19).
FIX: Build-require `perl(File::Basename)' (t/load_crontab.t:4).
FIX: Build-require `perl(strict)' (t/after_job.t:7).
FIX: Build-require `perl(warnings)' (t/delete_entry.t:12).

Test::Kwalitee, Test::Pod, Test::Pod::Coverage, DateTime::TimeZone::Local are an optional for tests. Ok.

Config and Term::ReadLine are not used at tests. Ok.

FIX: Build-require `perl(Data::Dumper)' (lib/Schedule/Cron.pm:65).
FIX: Build-require `perl(subs)' (lib/Schedule/Cron.pm:69).
FIX: Build-require `perl(vars)' (lib/Schedule/Cron.pm:68).

All tests pass. Ok.

$ rpmlint perl-Schedule-Cron.spec ../SRPMS/perl-Schedule-Cron-1.01-1.fc30.src.rpm ../RPMS/noarch/perl-Schedule-Cron-1.01-1.fc30.noarch.rpm 
perl-Schedule-Cron.src: W: spelling-error %description -l en_US crontab -> contactable
perl-Schedule-Cron.noarch: W: spelling-error %description -l en_US crontab -> contactable
2 packages and 1 specfiles checked; 0 errors, 2 warnings.
rpmlint is Ok.

$ rpm -q -lv -p ../RPMS/noarch/perl-Schedule-Cron-1.01-1.fc30.noarch.rpm 
drwxr-xr-x    2 root    root                        0 Dec 14 14:40 /usr/share/doc/perl-Schedule-Cron
-rw-r--r--    1 root    root                     3388 Jun  6  2011 /usr/share/doc/perl-Schedule-Cron/CHANGES
-rw-r--r--    1 root    root                     3948 Jun  6  2011 /usr/share/doc/perl-Schedule-Cron/ChangeLog
-rw-r--r--    1 root    root                     5412 Jun  6  2011 /usr/share/doc/perl-Schedule-Cron/README
drwxr-xr-x    2 root    root                        0 Jun  6  2011 /usr/share/doc/perl-Schedule-Cron/examples
-rw-r--r--    1 root    root                       95 Jun  6  2011 /usr/share/doc/perl-Schedule-Cron/examples/cron.tab
-rw-r--r--    1 root    root                    11633 Jun  6  2011 /usr/share/doc/perl-Schedule-Cron/examples/custom_sleep.pl
-rw-r--r--    1 root    root                      861 Jun  6  2011 /usr/share/doc/perl-Schedule-Cron/examples/simple.pl
drwxr-xr-x    2 root    root                        0 Dec 14 14:40 /usr/share/licenses/perl-Schedule-Cron
-rw-rw-r--    1 root    root                     6600 Dec 14 14:10 /usr/share/licenses/perl-Schedule-Cron/Licensing.html
-rw-r--r--    1 root    root                     9190 Dec 14 14:40 /usr/share/man/man3/Schedule::Cron.3pm.gz
drwxr-xr-x    2 root    root                        0 Dec 14 14:40 /usr/share/perl5/vendor_perl/Schedule
-rw-r--r--    1 root    root                    56146 Dec 14 14:40 /usr/share/perl5/vendor_perl/Schedule/Cron.pm
File layout and permissions is Ok.

$ rpm -q --requires -p ../RPMS/noarch/perl-Schedule-Cron-1.01-1.fc30.noarch.rpm | sort -f | uniq -c
      1 perl(Data::Dumper)
      1 perl(strict)
      1 perl(subs)
      1 perl(Time::ParseDate)
      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
FIX: Run-require `perl(Config)' (lib/Schedule/Cron.pm:1708).
FIX: Constrain `perl(Time::ParseDate)' run-dependency with `>= 2011.0505' (Makefile.PL:19).
TODO: POSIX is optional, declare the optional dependency using `Suggests: perl(POSIX)'.
TODO: Term::ReadLine is optional, declare the optional dependency using `Suggests: perl(Term::ReadLine)'.

$ rpm -q --provides -p ../RPMS/noarch/perl-Schedule-Cron-1.01-1.fc30.noarch.rpm | sort -f | uniq -c
      1 perl(Schedule::Cron) = 1.01
      1 perl-Schedule-Cron = 1.01-1.fc30
Binary provides are Ok.

$ resolvedeps rawhide ../RPMS/noarch/perl-Schedule-Cron-1.01-1.fc30.noarch.rpm 
Binary dependencies are resolvable. Ok.

The package builds in F30 (https://koji.fedoraproject.org/koji/taskinfo?taskID=31459631). Ok.

Otherwise the package is in line with Fedora and Perl packaging guide lines.
Please correct all `FIX' items, consider fixint `TODO' items and provide a new spec file.
Resolution: Package NOT approved.

Comment 3 Andrew Bauer 2018-12-14 18:46:15 UTC
Thank you for the feedback.

> TODO: Remove the obsolete `Group:' tag.
Done.

> FIX: Build-require `coreutils' (perl-Schedule-Cron.spec:57).
Done.

> TODO: Do not build-require `perl-devel`, it's not needed.
Done.

> TODO: Constrain `perl(ExtUtils::MakeMaker)' build-dependency with `>= 6.76' because of NO_PACKLIST=1 and remove `find %{buildroot} -type f -name .packlist -delete' line from the %install section. Or remove the `NO_PACKLIST=1' argument.
The NO_PACKLIST flag does not cause an error condition for older MakeMaker's, but I have chosen the second option since I plan to build for epel7.
Done.

> FIX: Remove `find %{buildroot} -type d -empty -delete' line from the %install section. ExtUtils::MakeMaker does not create empty directories, the command is not necessary.
Done. 

> FIX: Constrain `perl(Time::ParseDate)' build-dependency with `>= 2011.0505' (Makefile.PL:19).
Done.

> FIX: Build-require `perl(File::Basename)' (t/load_crontab.t:4).
Done.

> FIX: Build-require `perl(strict)' (t/after_job.t:7).
Done.

> FIX: Build-require `perl(warnings)' (t/delete_entry.t:12).
Done.

......

> FIX: Build-require `perl(Data::Dumper)' (lib/Schedule/Cron.pm:65).
Done.

> FIX: Build-require `perl(subs)' (lib/Schedule/Cron.pm:69).
Done.

> FIX: Build-require `perl(vars)' (lib/Schedule/Cron.pm:68).
Done.

......

> FIX: Run-require `perl(Config)' (lib/Schedule/Cron.pm:1708).
Done.

> FIX: Constrain `perl(Time::ParseDate)' run-dependency with `>= 2011.0505' (Makefile.PL:19).
Done.

> TODO: POSIX is optional, declare the optional dependency using `Suggests: perl(POSIX)'.
Used Requires, rather than Suggests, for el7 compatibility.
Done.

> TODO: Term::ReadLine is optional, declare the optional dependency using `Suggests: perl(Term::ReadLine)'.
Used Requires, rather than Suggests, for el7 compatibility.
Done.

Updated SPEC Url: https://raw.githubusercontent.com/knight-of-ni/specfiles/master/perl-Schedule-Cron.spec
Updated SRPM Url: https://copr-be.cloud.fedoraproject.org/results/kni/perl-Schedule-Cron/fedora-rawhide-x86_64/00837356-perl-Schedule-Cron/perl-Schedule-Cron-1.01-1.fc30.src.rpm

Comment 4 Petr Pisar 2018-12-17 10:00:26 UTC
The spec file changes:

--- perl-Schedule-Cron.spec.old 2018-12-14 14:09:48.827000000 +0100
+++ perl-Schedule-Cron.spec     2018-12-17 10:32:01.556000000 +0100
@@ -3,7 +3,6 @@
 Version:   1.01
 Release:   1%{?dist}
 License:   GPL+ or Artistic
-Group:     Development/Libraries
 URL:       https://metacpan.org/release/Schedule-Cron
 BuildArch: noarch
 
@@ -12,19 +11,34 @@
 # shown in Makefile.PL
 Source1:        http://dev.perl.org/licenses/#/%{name}-Licensing.html
 
-Patch0: https://github.com/rhuss/schedule-cron/pull/8.patch#/perl-schedule-cron-fix-unescaped-left-brace.patch
+# https://github.com/rhuss/schedule-cron/pull/8
+Patch0: perl-schedule-cron-fix-unescaped-left-brace.patch
 # Patch obtained from Debian libschedule-cron-perl source package
 Patch1: perl-schedule-cron-fix-spelling.patch
 
-BuildRequires:  perl-devel
 BuildRequires:  perl-interpreter
 BuildRequires:  perl-generators
 BuildRequires:  perl(ExtUtils::MakeMaker)
+BuildRequires:  perl(File::Basename)
+BuildRequires:  perl(strict)
+BuildRequires:  perl(warnings)
+BuildRequires:  perl(Data::Dumper)
+BuildRequires:  perl(subs)
+BuildRequires:  perl(vars)
 BuildRequires:  findutils
+BuildRequires:  coreutils
 
 # Needed during build for the perl test
-BuildRequires: perl(Time::ParseDate)
-BuildRequires: perl(Test::More)
+BuildRequires:  perl(Time::ParseDate) >= 2011.0505
+BuildRequires:  perl(Test::More)
+BuildRequires:  perl(Test::Pod)
+BuildRequires:  perl(Test::Pod::Coverage)
+BuildRequires:  perl(Test::Kwalitee)
+
+Requires:       perl(Config)
+Requires:       perl(Time::ParseDate) >= 2011.0505
+Requires:       perl(POSIX)
+Requires:       perl(Term::ReadLine)
 
 %description
 This module provides  a simple but complete cron  like scheduler.  I.e
@@ -57,14 +71,13 @@
 cp -a %{SOURCE1} Licensing.html
 
 %build
-%{__perl} Makefile.PL INSTALLDIRS=vendor NO_PACKLIST=1 NO_PERLLOCAL=1
+%{__perl} Makefile.PL INSTALLDIRS=vendor NO_PERLLOCAL=1
 %make_build
 
 %install
 %make_build pure_install DESTDIR=%{buildroot}
 
 find %{buildroot} -type f -name .packlist -delete
-find %{buildroot} -type d -empty -delete
 
 %{_fixperms} %{buildroot}/*

The changes are good.

TODO: Except I do not recommend running the optional tests with Test::Kwalitee or Test::Pod. They are very fragile and tends to break when somebody updates the modules (Test::Kwalitee is a list of subjective rules how a good Perl code looks like).

TODO: You should also build-require `perl(POSIX)' if you decided to hard-require it. It's good to assure the tests have a similar environment as the package has after installing on a user's system. Otherwise it can happen that the tests test a different code path than the user will perform (see all the $HAS_POSIX conditions in the code).

$ rpmlint perl-Schedule-Cron.spec ../SRPMS/perl-Schedule-Cron-1.01-1.fc30.src.rpm ../RPMS/noarch/perl-Schedule-Cron-1.01-1.fc30.noarch.rpm 
perl-Schedule-Cron.src: W: spelling-error %description -l en_US crontab -> contactable
perl-Schedule-Cron.noarch: W: spelling-error %description -l en_US crontab -> contactable
2 packages and 1 specfiles checked; 0 errors, 2 warnings.
rpmlint is Ok.

$ rpm -q --requires -p ../RPMS/noarch/perl-Schedule-Cron-1.01-1.fc30.noarch.rpm | sort -f | uniq -c
      1 perl(Config)
      1 perl(Data::Dumper)
      1 perl(POSIX)
      1 perl(strict)
      1 perl(subs)
      1 perl(Term::ReadLine)
      1 perl(Time::ParseDate)
      1 perl(Time::ParseDate) >= 2011.0505
      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
The binary requires are Ok.

TODO: You can filter out the non-versioned `perl(Time::ParseDate)' dependency as it's redundant and pollutes the metadata.

$ resolvedeps rawhide ../RPMS/noarch/perl-Schedule-Cron-1.01-1.fc30.noarch.rpm 
Binary dependencies are resolvable. Ok.

The package builds in F30 (https://koji.fedoraproject.org/koji/taskinfo?taskID=31507198). Ok.

Otherwise the package is good.
Please consider fixing the `TODO' items before building the package.
Resolution: The package is APPROVED.

Comment 5 Andrew Bauer 2018-12-18 02:08:22 UTC
> TODO: Except I do not recommend running the optional tests with Test::Kwalitee or Test::Pod. They are very fragile and tends to break when somebody updates the modules (Test::Kwalitee is a list of subjective rules how a good Perl code looks like).
Thank you. I never would have suspected this.
Done.

> TODO: You should also build-require `perl(POSIX)' if you decided to hard-require it. It's good to assure the tests have a similar environment as the package has after installing on a user's system. Otherwise it can happen that the tests test a different code path than the user will perform (see all the $HAS_POSIX conditions in the code).
Done.

> TODO: You can filter out the non-versioned `perl(Time::ParseDate)' dependency as it's redundant and pollutes the metadata.
I was not sure how to resolve this, since the non-versioned dependency was being automatically picked up by rpm. Google to the rescue.
That's when I discovered the __requires_exclude macro and added this to the top of the specfile:
%global __requires_exclude perl\\(Time::ParseDate\\)

That did it.

Thank you for your time.

Comment 6 Igor Raits 2018-12-18 13:41:48 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/perl-Schedule-Cron

Comment 7 Fedora Update System 2018-12-19 00:47:39 UTC
perl-Schedule-Cron-1.01-1.el7 has been pushed to the Fedora EPEL 7 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2018-1ff892923a

Comment 8 Fedora Update System 2018-12-19 01:43:45 UTC
perl-Schedule-Cron-1.01-1.fc28 has been pushed to the Fedora 28 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2018-4da7e24c1e

Comment 9 Fedora Update System 2018-12-19 03:36:51 UTC
perl-Schedule-Cron-1.01-1.fc29 has been pushed to the Fedora 29 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2018-e68c655b70

Comment 10 Fedora Update System 2018-12-27 01:50:51 UTC
perl-Schedule-Cron-1.01-1.fc28 has been pushed to the Fedora 28 stable repository. If problems still persist, please make note of it in this bug report.

Comment 11 Fedora Update System 2018-12-28 05:12:33 UTC
perl-Schedule-Cron-1.01-1.fc29 has been pushed to the Fedora 29 stable repository. If problems still persist, please make note of it in this bug report.

Comment 12 Fedora Update System 2019-01-03 17:44:54 UTC
perl-Schedule-Cron-1.01-1.el7 has been pushed to the Fedora EPEL 7 stable repository. If problems still persist, please make note of it in this bug report.


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