Bug 924750 - Review Request: perl-ExtUtils-ParseXS - Module and a script for converting Perl XS code into C code
Summary: Review Request: perl-ExtUtils-ParseXS - Module and a script for converting Pe...
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Petr Šabata
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2013-03-22 12:42 UTC by Petr Pisar
Modified: 2013-04-29 11:25 UTC (History)
3 users (show)

Fixed In Version: perl-ExtUtils-ParseXS-3.18-1.fc20
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2013-04-29 11:25:49 UTC
Type: ---
Embargoed:
psabata: fedora-review+


Attachments (Terms of Use)

Description Petr Pisar 2013-03-22 12:42:32 UTC
Spec URL: http://ppisar.fedorapeople.org/perl-ExtUtils-ParseXS/perl-ExtUtils-ParseXS.spec
SRPM URL: http://ppisar.fedorapeople.org/perl-ExtUtils-ParseXS/perl-ExtUtils-ParseXS-3.18-1.fc20.src.rpm
Description:
ExtUtils::ParseXS will compile XS code into C code by embedding the
constructs necessary to let C functions manipulate Perl values and creates
the glue necessary to let Perl access those functions.

Fedora Account System Username: ppisar

----
This package will dual-live with core perl modules.

Comment 1 Petr Šabata 2013-04-24 13:20:01 UTC
This is a noarch package, you don't need to define the OPTIMIZE variable.
Line 57 is also unnecessary.

Style nitpicking: capitalize 'requires' on line 37.

Even though META.yml lists ExtUtils::MakeMaker as a runtime dependency, I can't really tell how it's used.  Similarly, could you explain the perl-devel dependency?  This package doesn't do any actual C compiling.

Comment 2 Petr Pisar 2013-04-25 11:29:03 UTC
(In reply to comment #1)
> This is a noarch package, you don't need to define the OPTIMIZE variable.
> Line 57 is also unnecessary.
> 
> Style nitpicking: capitalize 'requires' on line 37.
> 
Fixed. 

> Even though META.yml lists ExtUtils::MakeMaker as a runtime dependency, I
> can't really tell how it's used.  Similarly, could you explain the
> perl-devel dependency?  This package doesn't do any actual C compiling.

perl-devel provides Perl header files that are needed by C files generated by this module to compile them. So I required perl-devel because there is currently no clear decision which packages should require it. (We use the same approach at ExtUtils-MakeMaker and Module-Build.)

Dependency on ExtUtils::MakeMaker is a remnant of META.yml probably. I know there are same disputes how MakeMaker uses ParseXS, so that could have been the reason.

What's your opinion? Should I drop the perl-devel and EU::MM run-time dependencies?

Comment 3 Petr Šabata 2013-04-25 14:21:06 UTC
(In reply to comment #2)
> (In reply to comment #1)
> > This is a noarch package, you don't need to define the OPTIMIZE variable.
> > Line 57 is also unnecessary.
> > 
> > Style nitpicking: capitalize 'requires' on line 37.
> > 
> Fixed. 

Ack.  Approving.

> > Even though META.yml lists ExtUtils::MakeMaker as a runtime dependency, I
> > can't really tell how it's used.  Similarly, could you explain the
> > perl-devel dependency?  This package doesn't do any actual C compiling.
> 
> perl-devel provides Perl header files that are needed by C files generated
> by this module to compile them. So I required perl-devel because there is
> currently no clear decision which packages should require it. (We use the
> same approach at ExtUtils-MakeMaker and Module-Build.)

I guess the correct way to handle this would be BR'ing perl-devel in every XS perl package.  Well, that's not the situation we're in at the moment so I suppose having perl-devel required here is a good thing... for now.

> Dependency on ExtUtils::MakeMaker is a remnant of META.yml probably. I know
> there are same disputes how MakeMaker uses ParseXS, so that could have been
> the reason.
> 
> What's your opinion? Should I drop the perl-devel and EU::MM run-time
> dependencies?

I'd drop EU::MM from the runtime dep list.

Comment 4 Petr Pisar 2013-04-25 14:46:11 UTC
I've updated the package not to run-require EU::MM.

Comment 5 Petr Šabata 2013-04-26 07:30:13 UTC
--- a/perl-ExtUtils-ParseXS.spec
+++ b/perl-ExtUtils-ParseXS.spec
@@ -35,7 +35,6 @@ BuildRequires:  perl(overload)
 BuildRequires:  perl(Test::More) >= 0.47
 Requires:       perl(:MODULE_COMPAT_%(eval "`perl -V:version`"; echo $version))
 Requires:       perl-devel
-Requires:       perl(ExtUtils::MakeMaker) >= 6.46
 # perl-ExtUtils-Typemaps has been merged into perl-ExtUtils-ParseXS, bug #891952
 Obsoletes:      perl-ExtUtils-Typemaps

Comment 6 Petr Pisar 2013-04-26 07:39:07 UTC
New Package SCM Request
=======================
Package Name: perl-ExtUtils-ParseXS
Short Description: Module and a script for converting Perl XS code into C code
Owners: ppisar jplesnik psabata
Branches: 
InitialCC: perl-sig

Comment 7 Gwyn Ciesla 2013-04-26 11:21:51 UTC
Unretired.

Comment 8 Petr Pisar 2013-04-26 13:47:56 UTC
Waiting on unblocking the package <https://fedorahosted.org/rel-eng/ticket/5594>.

Comment 9 Petr Pisar 2013-04-29 11:25:49 UTC
Thank you for the review and repository.


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