Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 175605
Summary: | Review Request: perl-IO-Multiplex | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Leif O M Bergman <lmb> |
Component: | Package Review | Assignee: | Greg DeKoenigsberg <gdk> |
Status: | CLOSED CURRENTRELEASE | QA Contact: | David Lawrence <dkl> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | fedora-package-review, nicolas.mailhot, wtogami |
Target Milestone: | --- | ||
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | 1.08-4 | Doc Type: | Bug Fix |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2005-12-28 14:54:50 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: | |||
Bug Depends On: | |||
Bug Blocks: | 163779 |
Description
Leif O M Bergman
2005-12-13 09:37:50 UTC
NEEDSWORK Blockers: * License is Artistic only. * Use "%check" instead of "%check ||:" in spec. Non blockers * Get rid of the %define rname at the beginning of the spec. * Use of "by-modules" CPAN URLs is discouraged, better use an "authors/id" URL, e.g. Source: http://www.cpan.org/authors/id/B/BB/BBB/IO-Multiplex-1.08.tar.gz Problematic: The package conditionally depends on Time::HiRes I've asked the authors of they can change the license. The spec file has been cleaned up. About the part that is deemed problematic... The code snippet looks like this: BEGIN { eval { # Can optionally use Hi Res timers if available require Time::HiRes; Time::HiRes->import ('time'); } }; I really don't see a problem with this. BTW, perl-IO-Multiplex is eval-required by the allready included perl-Net-Server package. It's needed by postgrey, the real reason for this excercise. (In reply to comment #2) > I've asked the authors of they can change the license. > The spec file has been cleaned up. Could it be that you miss-read my remark? The Artistic license is fine for includion into FE, it's just that your spec file said "GPL or Artistic", which is apparently wrong. > About the part that is deemed problematic... The code snippet looks like this: > BEGIN { > eval { > # Can optionally use Hi Res timers if available > require Time::HiRes; > Time::HiRes->import ('time'); > } > }; > > I really don't see a problem with this. The problem is: Conditional requires, like this one lead to non-deterministic "make check" results, and can cause Heisenbugs, depending on if the conditional module (here: Time::HiRes) is installed or not. This isn't much of a problem (Therefore I wrote "Non-Blocker"), but can be problematic in longer terms. One way to circumvent this kind of problems would be to make this conditional dependencies mandatory at run-time (Require: perl(Time::HiRes)) or at build-time ("BuildRequires: perl(Time::HiRes)", even "BuildConflicts: perl(Time::HiRes)" could be worth a thought). The decision on what to do about it is up to you. Even not doing anything about it and leaving things as they currently are is absolutely fine. Yup, misread you but fixed the problem all the same. ;-) For the time being I'll leave the perl run-time dependency as is. After all, I'm allready chasing another one. So, one dependecy at a time... Could you check again: Spec Name or Url: http://www.biosci.ki.se/users/lmb/SRPMS/perl-IO-Multiplex.spec SRPM Name or Url: http://www.biosci.ki.se/users/lmb/SRPMS/perl-IO-Multiplex-1.08-4.src.rpm Forgot to mention that this is my first package and that I need a sponsor. (In reply to comment #4) > http://www.biosci.ki.se/users/lmb/SRPMS/perl-IO-Multiplex-1.08-4.src.rpm Package seems OK to me - APPROVED (In reply to comment #5) > Forgot to mention that this is my first package and that I need a sponsor. Please follow Step 8 in http://fedoraproject.org/wiki/Extras/Contributors Ralf do you intend on sponsoring Leif? Otherwise Leif, http://fedoraproject.org/wiki/Extras/Contributors Candidates are eligible for cvsextras access only after they have approved packages in the Fedora Extras review process. After this point sponsors may choose to sponsor your cvsextras membership by judging your knowledge and understanding of the packaging guidelines and project processes. A good way to convince sponsors of your knowledge is to continue with other packages, or to review packages submitted by other users. There is no better way to convince sponsors that you have read and understand the packaging guidelines than to give good advice to other contributors about how they can improve their packages. http://fedoraproject.org/wiki/Extras If you have any questions about Fedora Extras, please see our extensive documention here first, then join fedora-extras-list and ask questions there. Is there any policy for splitting perl packages into subpackages? I see in the http://fedoraproject.org/wiki/PackageNamingGuidelines that splitting a CPAN module is a viable option. But how important is dependency granularity? Check this: https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=174099 So far nothing in FE has been borked by this. That also means that nothing else in FE is depending on perl-IO-Multiplex. So my opinion is that a subpackage is in order in this case. But I'm ready to be convinced otherwise... ie: perl-Net-Server ^ | (subpackage split) | perl-Net-Server-Multiplex -> perl-IO-Multiplex The alternative is to just add a Requires: perl-IO-Multiplex in perl-Net-Server. Which alternative is preferable? On a different note, who is maintaing postfix (in core)? And what would the implications be for mucking with master.cf and main.cf? I would like to have scriptlets in postfix-postgrey that modify those files to add funtionality. Much the way as the kernel plays with grub.cong. But the modification of smtpd_recipient_restrictions would be left as an excercise for root. ;-) |