Bug 189452 - Review Request: perl-Curses - Curses bindings for perl
Review Request: perl-Curses - Curses bindings for perl
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jochen Schmitt
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-04-19 23:01 EDT by Garrick Staples
Modified: 2009-10-21 12:23 EDT (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-04-24 16:02:09 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Garrick Staples 2006-04-19 23:01:34 EDT
Spec URL: http://www-rcf.usc.edu/~garrick/perl-Curses.spec
SRPM URL: http://www-rcf.usc.edu/~garrick/perl-Curses-1.13-1.src.rpm
Description: Perl bindings for ncurses, bringing terminal-independant character handling capabilities to Perl.
Comment 1 Jochen Schmitt 2006-04-20 10:55:22 EDT
Good:

+ Local buid works
+ Mock build works fine


Bad:
- Source0 should contain a full-qualified URL
- Perl should be not included as Buildrequires.
- The package should contain the text of the license
Comment 2 Jose Pedro Oliveira 2006-04-20 11:25:57 EDT
Needswork:
* the license should be "GPL or Artistic". Check the README file.
* add the following doc files "Copying Artistic README"
  (the license files were already mentioned)
* add the demo scripts as doc 

Others improvements:
* use the following url instead
  http://search.cpan.org/dist/Curses/
  as it's much more useful
Comment 3 Garrick Staples 2006-04-20 13:17:07 EDT
Thanks for the feedback, I think I have everything fixed up.

I wasn't entirely sure how demo scripts should be included in %doc because that
triggers rpmlint.  I corrected #!/usr/local/bin/perl and removed execute bits.

http://www-rcf.usc.edu/~garrick/perl-Curses.spec
http://www-rcf.usc.edu/~garrick/perl-Curses-1.13-2.src.rpm
Comment 4 Jochen Schmitt 2006-04-20 14:43:33 EDT
From my point of you, you may put the demo scripts into %doc and ignore the
rpmlint messages.
Comment 5 Garrick Staples 2006-04-21 02:31:55 EDT
Is 1.13-2 acceptable in its current form, or should I not remove the execute bits?
Comment 6 Paul Howarth 2006-04-21 12:45:34 EDT
(In reply to comment #5)
> Is 1.13-2 acceptable in its current form, or should I not remove the execute bits?

You could leave the exec bits on. The extra dependencies you get as a result of
this are:

 * /usr/bin/perl
 * perl(Curses)
 * perl(ExtUtils::testlib)

perl(Curses) is provided by the package itself, and the other two are provided
by the main perl package, which is already a dependency of this package. So
there aren't any new dependencies introduced by leaving the exec bits on.

It might also be worth adding a comment in the spec file that the
worrying-looking output from the build script...:

Making a guess for "c-config.h"...
WARNING: Your Curses form.h file appears to be in the default
system search path, which will not work for us because of
the conflicting Perl form.h file.  This means your 'make' will
probably fail unless you fix this, as described in the INSTALL
file.

... can be ignored because /usr/include/form.h is a symlink to
/usr/include/ncurses/form.h, which the Makefile.PL finds and uses quite happily.
Comment 7 Garrick Staples 2006-04-21 16:19:35 EDT
Put the exec bits back and added a note about the warning.

http://www-rcf.usc.edu/~garrick/perl-Curses.spec
http://www-rcf.usc.edu/~garrick/perl-Curses-1.13-3.src.rpm

Comment 8 Jose Pedro Oliveira 2006-04-22 18:18:09 EDT
(In reply to comment #6)
> (In reply to comment #5)
> > Is 1.13-2 acceptable in its current form, or should I not remove the execute
bits?
> 
> You could leave the exec bits on. The extra dependencies you get as a result of
> this are:
> 
>  * /usr/bin/perl
>  * perl(Curses)
>  * perl(ExtUtils::testlib)
> 
> perl(Curses) is provided by the package itself, and the other two are provided
> by the main perl package, which is already a dependency of this package. So
> there aren't any new dependencies introduced by leaving the exec bits on.

1) IIRC there are plans to disallowed this in the future (no docs with execution
permissions). Even rpmlint already warns about requirements being pulled in by
doc files:

    W: perl-Curses doc-file-dependency ...

2) Right now the only way to avoid the requirements pulled in by perl modules 
(.pm files) shipped as doc is to filter them out.

jpo
Comment 9 Jose Pedro Oliveira 2006-04-22 18:30:34 EDT
PUBLISH++

MD5SUMS:
4cf5520405d5a24fac3a4b3d60db516d  perl-Curses-1.13-3.src.rpm

33f6a17f9ece7efda2dde3431e1540f6  Curses-1.13.tgz
07deea06fa73cb099cf7d77a8e5d3b8b  perl-Curses.spec

Good:
* CPAN tarball MD5 digest matches the one included in the SRPM
* URL and Source URL are valid
* License verified (README file)
* Build Requirements are correct
* perl(:MODULE_COMPAT_xxx) present
* perl vendor libs present
* Builds without problems in FC-3 and FC-5
* (Un)installs without problems in FC-3 and FC-5
* The demo files run correctly
* No relevant bugs http://rt.cpan.org/NoAuth/Bugs.html?Dist=Curses

Minor notes:
* the comment in the %files section could/should be dropped
* see previous comment about executable doc files


This a my vote for publishing this package. As this package has several
reviewers, the final approval should be done by Jochen Schmitt (the first reviewer).

jpo
Comment 10 Garrick Staples 2006-04-22 19:03:20 EDT
As long as the reviewers are disagreeing with each other, I'll throw in my own
opinion :)

In the interest of maintaining a strict seperation of code and data, %doc files
should not be executable.  Whoever added the "doc-file-dependency" warning to
rpmlint was wise, and should take the next step of printing an error for exec bits.
Comment 11 Jochen Schmitt 2006-04-23 14:27:45 EDT
QStapler,

from my view the demo files should be contains in the %doc section, becouse 
the aint of this file is to demonstrate the use of the package.
Comment 12 Jochen Schmitt 2006-04-23 15:05:04 EDT
Thank you Petro for the Co-Review.

Form my point the package is APPROVED.
Comment 13 Paul Howarth 2006-04-24 10:11:40 EDT
(In reply to comment #8)
> (In reply to comment #6)
> > (In reply to comment #5)
> > > Is 1.13-2 acceptable in its current form, or should I not remove the execute
> bits?
> > 
> > You could leave the exec bits on. The extra dependencies you get as a result of
> > this are:
> > 
> >  * /usr/bin/perl
> >  * perl(Curses)
> >  * perl(ExtUtils::testlib)
> > 
> > perl(Curses) is provided by the package itself, and the other two are provided
> > by the main perl package, which is already a dependency of this package. So
> > there aren't any new dependencies introduced by leaving the exec bits on.
> 
> 1) IIRC there are plans to disallowed this in the future (no docs with execution
> permissions). Even rpmlint already warns about requirements being pulled in by
> doc files:
> 
>     W: perl-Curses doc-file-dependency ...
> 
> 2) Right now the only way to avoid the requirements pulled in by perl modules 
> (.pm files) shipped as doc is to filter them out.

My point was that although there are "logically" additional deps from having the
executable deps, in practice there are in fact no additional deps *in this case*
because the same packages that satisfy the main deps also satisfy the "doc"
deps. Of course that could change if some modules were split out from the main
perl package.

If at some time in the future rpm doesn't allow executable docs, that's also
fine by me, as is this package, with or without the executable docs.
Comment 14 Garrick Staples 2006-04-24 16:01:25 EDT
Thanks for approval.  The package should be showing up on mirrors soon.
Comment 15 Christian Iseli 2006-12-30 18:32:38 EST
Properly block FE-ACCEPT
Comment 16 Steve Traylen 2009-09-29 02:40:01 EDT

Package Change Request
======================
Package Name: perl-Curses
Owners: stevetraylen
New Branches: EL-4 EL-5


I emailed Garrick requesting that this package also be built 
for EPEL to which he responded:

How would you feel about taking it over as the maintainer?
HPCC/Linux Systems Admin
Garrick


I am happy to become the owner of this package and then also 
create the EL-4, EL-5 branch.
I think such a CVS request is also the way to change package 
ownership.

Steve
Comment 17 Kevin Fenzi 2009-09-29 16:26:27 EDT
cvs done on the EL-5/EL-4 branches. 

For the rest, Garrick should be able to approve you at
https://admin.fedoraproject.org/pkgdb/packages/name/perl-Curses
Comment 18 Fedora Update System 2009-09-30 14:10:55 EDT
perl-Curses-1.27-4.el5 has been submitted as an update for Fedora EPEL 5.
http://admin.fedoraproject.org/updates/perl-Curses-1.27-4.el5
Comment 19 Fedora Update System 2009-09-30 14:11:36 EDT
perl-Curses-1.27-3.el4 has been submitted as an update for Fedora EPEL 4.
http://admin.fedoraproject.org/updates/perl-Curses-1.27-3.el4
Comment 20 Fedora Update System 2009-10-21 12:23:21 EDT
perl-Curses-1.27-3.el4 has been pushed to the Fedora EPEL 4 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 21 Fedora Update System 2009-10-21 12:23:51 EDT
perl-Curses-1.27-4.el5 has been pushed to the Fedora EPEL 5 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.