Bug 483390

Summary: Review Request: perl-Schedule-Cron-Events - Take a line from a crontab and find out when events will occur
Product: [Fedora] Fedora Reporter: Robert Scheck <redhat-bugzilla>
Component: Package ReviewAssignee: manuel wolfshant <manuel.wolfshant>
Status: CLOSED DUPLICATE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: low    
Version: rawhideCC: fedora-package-review, msuchy, notting, tcallawa
Target Milestone: ---Flags: manuel.wolfshant: fedora-review+
kevin: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2009-02-10 23:38:37 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Robert Scheck 2009-01-31 13:36:26 UTC
Spec URL: http://labs.linuxnetz.de/bugzilla/perl-Schedule-Cron-Events.spec
SRPM URL: http://labs.linuxnetz.de/bugzilla/perl-Schedule-Cron-Events-1.8-1.src.rpm
Description: Given a line from a crontab, tells you the time at which cron
will next run the line or when the last event occurred, relative to any
date you choose. And this module uses Set::Crontab to understand the date 
specification, so it should be able to handle all forms of cron entries.

Comment 1 manuel wolfshant 2009-01-31 17:50:05 UTC
Robert, the fedora‑review flag must be set by the reviewer...

Comment 2 Robert Scheck 2009-01-31 18:08:14 UTC
Of course, Manuel. Sorry, that happened accidentally.

Comment 3 manuel wolfshant 2009-01-31 18:58:37 UTC
Suggestion: drop the BR on perl> 5.8.0.

MUSTFIX: drop the perldoc -t lines. We are not allowed to add licenses to packages which do not ship them [*]. Instead you should contact the author and ask him to include them. Note that cron_event_predict.plx has no license at all and this should be clarified before inclusion.


Quote from ReviewGuidelines:
MUST: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package must be included in %doc.[4]

Comment 4 Robert Scheck 2009-01-31 19:31:26 UTC
Interesting: In bug #193960 comment #6, I was even asked to add such a file
to avoid confusions. I'm no lawyer, but the text from the Review Guidelines 
seems not to forbid my behaviour, it only forces the other way round.

Regarding cron_event_predict.plx: It mostly contains documentation, the rest
is "just" example code. So let's clarify both points you've mentioned now by 
someone from Fedora Legal...

Comment 5 Tom "spot" Callaway 2009-01-31 19:45:46 UTC
You can add the license text(s) if you're comfortable, but you are by no means required to do so. You should advise upstream to include them, as their omission is a notable bug in the software.

As to cron_event_predict.plx, you should confirm with upstream that it is available under the same terms as the library, because the licensing attribution only covers the library:

"This library is free software; you can redistribute it and/or modify it under the same terms as Perl itself."

I'm leaving FE-Legal here until the licensing on cron_event_predict.plx is clarified or it is removed.

Comment 6 manuel wolfshant 2009-01-31 20:54:38 UTC
As far as I can see, the Review Guideline say "if (and only if)...". I admit that my English is not so good, but I do not think that my understanding of "and only if" is wrong... Is there a special meaning due to the fact that those three words are in parenthesizes ?

Comment 7 Tom "spot" Callaway 2009-02-02 14:47:07 UTC
(In reply to comment #6)
> As far as I can see, the Review Guideline say "if (and only if)...". I admit
> that my English is not so good, but I do not think that my understanding of
> "and only if" is wrong... Is there a special meaning due to the fact that those
> three words are in parenthesizes ?

Hmm, I can see how that is unclear. As the author of those words, I can assure you that this is what I intended them to mean:

When the source package includes the text of the license(s) in separate file(s), then that file or files, containing the text of the license(s) for the package must be included in %doc. If they are not present, the package maintainer may add copies of the licenses in the Fedora package, but is not required to do so. However, in such cases, the packager should contact upstream and encourage them to correct this mistake. 

^^^ Does that make things clearer? If so, I'll propose the change to FPC.

Comment 8 manuel wolfshant 2009-02-02 16:53:35 UTC
Oh, yes, it's definitely more clear. And it has a different meaning than what I have understood from it. Although I still find a bit awkward...


Package Review
==============

Key:
 - = N/A
 x = Check
 ! = Problem
 ? = Not evaluated

=== REQUIRED ITEMS ===
 [x] Package is named according to the Package Naming Guidelines.
 [x] Spec file name must match the base package %{name}, in the format %{name}.spec.
 [x] Package meets the Packaging Guidelines including the Perl specific items
 [x] Package successfully compiles and builds into binary rpms on at least one supported architecture.
     Tested on: devel/x86_64
 [x] Rpmlint output:
source RPM: empty
binary RPM:empty
 [x] Package is not relocatable.
 [x] Buildroot is correct (%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n))
 [x] Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines.
 [x] License field in the package spec file matches the actual license.
     License type: GPL+ or Artistic (same as perl)
=> see also note 1
 [x] If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %doc.
 [x] Spec file is legible and written in American English.
 [x] Sources used to build the package matches the upstream source, as provided in the spec URL.
     SHA1SUM of package: cd82796797be1aba3e9102af9a3023feada104c6 Schedule-Cron-Events-1.8.tar.gz
 [x] Package is not known to require ExcludeArch
 [x] All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines.
 [-] The spec file handles locales properly.
 [-] ldconfig called in %post and %postun if required.
 [x] Package must own all directories that it creates.
 [x] Package requires other packages for directories it uses.
 [x] Package does not contain duplicates in %files.
 [x] Permissions on files are set properly.
 [x] Package has a %clean section, which contains rm $RPM_BUILD_ROOT.
 [x] Package consistently uses macros.
 [x] Package contains code, or permissable content.
 [-] Large documentation files are in a -doc subpackage, if required.
 [x] Package uses nothing in %doc for runtime.
 [-] Header files in -devel subpackage, if present.
 [-] Static libraries in -devel subpackage, if present.
 [-] Package requires pkgconfig, if .pc files are present.
 [-] Development .so files in -devel subpackage, if present.
 [-] Fully versioned dependency in subpackages, if present.
 [x] Package does not contain any libtool archives (.la).
 [-] Package contains a properly installed %{name}.desktop file if it is a GUI application.
 [x] Package does not own files or directories owned by other packages.

=== SUGGESTED ITEMS ===
 [x] Latest version is packaged.
 [x] Package does not include license text files separate from upstream.
 [-] Description and summary sections in the package spec file contains translations for supported Non-English la
nguages, if available.
 [x] Reviewer should test that the package builds in mock.
     Tested on: devel/x86_64
 [x] Package should compile and build into binary rpms on all supported architectures.
     Tested on: devel/x86_64
 [?] Package functions as described.
 [-] Scriptlets must be sane, if used.
 [-] The placement of pkgconfig(.pc) files is correct.
 [-] File based requires are sane.
 
Note:
I still recommend dropping the BR on perl> 5.8.0. Both Fedora and RHEL 4/5 have much newer versions. Your call.

================
*** APPROVED ***
================

Comment 9 manuel wolfshant 2009-02-02 17:02:51 UTC
oh well, note 1 would have said (if I wouldn't pasted over the selection): "do not forget to ask upstream to include the licenses"

Comment 10 Robert Scheck 2009-02-02 17:06:29 UTC
Maybe approved but we've still the legal blocker regarding *.plx. I will
clarify this first before any CVS import - if upstream replies to me.

Tom, would it also be suitable not to ship the *.plx in the binary RPM, but
just in the SRPM package? Or would I have to remove the *.plx then from the
tarball as well?

Comment 11 Tom "spot" Callaway 2009-02-02 17:55:35 UTC
Well, given that we don't know if we have permission to redistribute the .plx file or not, I'm going to say that you need a modified tarball.

Comment 12 Robert Scheck 2009-02-10 15:12:39 UTC
New Package CVS Request
=======================
Package Name: perl-Schedule-Cron-Events
Short Description: Take a line from a crontab and find out when events will occur
Owners: robert
Branches: EL-4 EL-5 F-9 F-10


I'll make a modified tarball without the *.plx file as upstream didn't reply
until now. Thus we're fine from Fedora Legal aspects.

Comment 13 Kevin Fenzi 2009-02-10 22:44:19 UTC
cvs done.

Comment 14 Kevin Fenzi 2009-02-10 22:48:13 UTC
Uh... this package was already in the collection. 

It's owned by msuchy. I have switched it back to them. 
Can you see if the legal concerns are answered in the already existing package?

Comment 15 Robert Scheck 2009-02-10 23:38:37 UTC
The legal concerns are neither answered nor solved in the already existing 
package. I've applied the changes from my preparations to CVS now.

Sorry, I somehow overlooked the package in the list. Nevertheless the review
has been a success in improving quality in Fedora. And Miroslav, I'm very
sorry about my mistake - luckily Kevin has corrected my wrong requests... :)

Comment 16 Miroslav Suchý 2009-02-11 10:35:01 UTC
Marking as DUPLICATE rather then CLOSED RAWHIDE to better track that this BZ is indeed duplicate.

*** This bug has been marked as a duplicate of bug 461736 ***

Comment 17 Miroslav Suchý 2009-02-11 10:49:43 UTC
I contacted upstream author to clarify this doubts and if possible release new version with LICENSE file.
If possible track further discussion under BZ 461736.