Bug 184530 - (perl-RPM2) Review Request: perl-RPM2
Review Request: perl-RPM2
Status: CLOSED CURRENTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Robin Norwood
Fedora Package Reviews List
: Reopened
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-03-09 12:40 EST by Jason Vas Dias
Modified: 2008-12-14 00:40 EST (History)
6 users (show)

See Also:
Fixed In Version: F7
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-08-25 16:11:58 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Jason Vas Dias 2006-03-09 12:40:59 EST
Spec Name or Url: http://people.redhat.com/~jvdias/perl-RPM2
SRPM Name or Url: http://people.redhat.com/~jvdias/perl-RPM2-0.66-12.src.rpm

Description:

perl-RPM2 was provided in all previous versions of RHEL / Fedora Core, but 
in FC-5 has been deprecated for some reason .

Here's a request to add it to Fedora Extras.
Comment 1 Jason Vas Dias 2006-03-09 12:42:15 EST
Spec Name or Url: http://people.redhat.com/~jvdias/perl-RPM2.spec
Comment 2 Paul Howarth 2006-03-14 10:59:57 EST
A few comments after a cursory look:

- I can't find any evidence of license terms under which this package can be
distributed, which would be a blocker and in need of fixing upstream
- Extras packages shouldn't have a buildreq on perl
- latest changelog release tag doesn't match actual release tag
- some of the tests fail but "make check" passes - why?
- missing buildreqs elfutils-libelf-devel and bzip2-devel
- using find/filelist method for %files results in unowned directory
%{perl_vendorarch}/auto/RPM2
- consider adding %{?dist} tag if this package is likely to be needed for future
Fedora releases
Comment 3 Jason Vas Dias 2006-03-16 16:32:08 EST
OK, I've resolved each of the issues raised:

>  - I can't find any evidence of license terms under which this package can be
>  distributed, which would be a blocker and in need of fixing upstream

The rpm has always had tag:
'License: distributable
'
and there are no license restrictions or copyright in any source file 
or on the package CPAN home-page. As perl-RPM2 is already distributed
in every Fedora Core and  RHEL release except FC-5, these terms are 
evidently acceptable. 

> - Extras packages shouldn't have a buildreq on perl

This package cannot be built without the perl package being
installed; hence, the 'BuildRequires: perl' tag.

> - latest changelog release tag doesn't match actual release tag

huh? The %{version}-%{release} is 0.66-12 - latest changelog:
* Thu Mar 08 2006 Jason Vas Dias <jvdias@redhat.com> - 0.66-12

> - some of the tests fail but "make check" passes - why?

Because the package was not being built as root. 

Now, all tests that require root access are skipped if being run by a non-root
user, so all tests pass as a non-root user; I've also tested that those tests
are run and pass if run by root.

> - missing buildreqs elfutils-libelf-devel and bzip2-devel

Added.

> - using find/filelist method for %files results in unowned directory
> %{perl_vendorarch}/auto/RPM2

I've added:
%dir %{perl_vendorarch}/auto/RPM2

to %files list.

>- consider adding %{?dist} tag if this package is likely to be needed for 
>  future Fedora releases

Added.

Modified .spec file and srpm at:
  http://people.redhat.com/~jvdias/perl-RPM2.spec
  http://people.redhat.com/~jvdias/perl-RPM2-0.66-12.src.rpm
Comment 4 Paul Howarth 2006-03-17 07:02:29 EST
(In reply to comment #3)
> OK, I've resolved each of the issues raised:
> 
> >  - I can't find any evidence of license terms under which this package can be
> >  distributed, which would be a blocker and in need of fixing upstream
> 
> The rpm has always had tag:
> 'License: distributable
> '
> and there are no license restrictions or copyright in any source file 
> or on the package CPAN home-page. As perl-RPM2 is already distributed
> in every Fedora Core and  RHEL release except FC-5, these terms are 
> evidently acceptable.

Put it this way; if this package had not been included in previous Core/RHEL
releases, it would definitely not be allowed into Extras without a clear
license. There is a similar issue affecting Bug 171640
(perl-Log-Dispatch-FileRotate). Given that packages migrating from Core to
Extras must go through the same review process as any other package, I don't
think this issue can waived so lightly.

What I think needs to happen is one of:
(a) Upstream clarifying the license, or
(b) Legal stating that this particular package is OK (and, preferably, why), or
(c) Legal stating that a class of packages into which this one falls (e.g.
packages from CPAN, packages migrating from Core) are OK to be included without
a clear license; this could be beneficial for perl-Log-Dispatch-FileRotate too.

> > - Extras packages shouldn't have a buildreq on perl
> 
> This package cannot be built without the perl package being
> installed; hence, the 'BuildRequires: perl' tag.

Perl is explicitly listed as one of the exclusions from BuildRequires in the
packaging guidelines at: http://fedoraproject.org/wiki/Packaging/Guidelines and
the package review guidelines at
http://fedoraproject.org/wiki/Packaging/ReviewGuidelines state that:

  MUST: A package must not contain any BuildRequires that are listed in
  the exceptions section of Packaging Guidelines.

> > - latest changelog release tag doesn't match actual release tag
> 
> huh? The %{version}-%{release} is 0.66-12 - latest changelog:
> * Thu Mar 08 2006 Jason Vas Dias <jvdias@redhat.com> - 0.66-12

Not in the original SRPM, which had:
* Fri Feb 03 2006 Jason Vas Dias <jvdias@redhat.com> - 0.66-11.1

> > - some of the tests fail but "make check" passes - why?
> 
> Because the package was not being built as root. 
> 
> Now, all tests that require root access are skipped if being run by a non-root
> user, so all tests pass as a non-root user; I've also tested that those tests
> are run and pass if run by root.

Good; I understand why the tests were failing originally, but what I don't
understand is why the presence of failing tests didn't cause the package build
to fail in %check.

> > - missing buildreqs elfutils-libelf-devel and bzip2-devel
> 
> Added.

Good. Build in mock now works.

> > - using find/filelist method for %files results in unowned directory
> > %{perl_vendorarch}/auto/RPM2
> 
> I've added:
> %dir %{perl_vendorarch}/auto/RPM2
> 
> to %files list.

Good.

> >- consider adding %{?dist} tag if this package is likely to be needed for 
> >  future Fedora releases
> 
> Added.

Good.

> Modified .spec file and srpm at:
>   http://people.redhat.com/~jvdias/perl-RPM2.spec
>   http://people.redhat.com/~jvdias/perl-RPM2-0.66-12.src.rpm

Please bump the release number for iterations of packages during the review
process; it helps people to see which version is being referred to.

Still to do: clarify license, remove perl buildreq.
Then I can approve.
Comment 5 Ville Skyttä 2006-03-17 11:59:36 EST
(In reply to comment #4)
>   MUST: A package must not contain any BuildRequires that are listed in
>   the exceptions section of Packaging Guidelines.

That MUST has been debated before, and especially in case of perl module
packages buildrequring perl.  Blocking packages based on that is IMHO a bit
silly, and yes, so is the guideline and I don't have anything against fixing it.
Comment 6 Paul Howarth 2006-03-17 12:05:40 EST
(In reply to comment #5)
> (In reply to comment #4)
> >   MUST: A package must not contain any BuildRequires that are listed in
> >   the exceptions section of Packaging Guidelines.
> 
> That MUST has been debated before, and especially in case of perl module
> packages buildrequring perl.  Blocking packages based on that is IMHO a bit
> silly, and yes, so is the guideline and I don't have anything against fixing it.

I agree that it's a silly MUST but the right the to do is to change the rule
rather than ignore it.

I would suggest the following:

Instead of:
 - MUST: A package must not contain any BuildRequires that are listed in the
exceptions section of Packaging Guidelines.
- MUST: All other Build dependencies must be listed in BuildRequires.

I'd have:
 - MUST: All build dependencies must be listed in BuildRequires, except for any
that are listed in the exceptions section of Packaging Guidelines; inclusion of
those as BuildRequires is optional.
Comment 7 Chris Weyl 2006-05-12 11:35:00 EDT
Looks like Packaging/ReviewGuidelines has been updated to deal with this:

- MUST: All build dependencies must be listed in BuildRequires, except for any
that are listed in the exceptions section of Packaging Guidelines; inclusion of
those as BuildRequires is optional. Apply common sense.
Comment 8 Paul Howarth 2006-05-12 11:47:15 EDT
(In reply to comment #7)
> Looks like Packaging/ReviewGuidelines has been updated to deal with this:
> 
> - MUST: All build dependencies must be listed in BuildRequires, except for any
> that are listed in the exceptions section of Packaging Guidelines; inclusion of
> those as BuildRequires is optional. Apply common sense.

Yes, that's OK, which just leaves the license issue to be resolved.
Comment 9 Jose Pedro Oliveira 2006-05-12 12:28:27 EDT
A couple of moths ago I opened two tickets in rt.cpan in order to ask Chip Turner
to add license information to RPM2 and Rpm::Specfile. No answer until now.  IIRC
in the past Warren was able to contact Chip. Maybe we should ping him.

By the way, isn't RPM2 included in the new versions of RPM? 
Comment 10 Jose Pedro Oliveira 2006-05-12 12:42:30 EDT
I found the following perl related lines in the CHANGES file of RPM 4.4.6

--------
4.4.5 -> 4.4.6:
  ...
  - perl: fix exuberant cut-n-paste.
  ...

4.4.4 -> 4.4.5:
  ...
  - fix: perldeps and prov.pl not to emit perl(main) (#177960)
  ...

4.4.3 -> 4.4.4:
  ...
  - perl: diddle Makefile.PL to build/link within rpm source tree.
  - perl: add build dependency on perl(ExtUtils::MakeMaker) >= 6.17.
  - complete forking perl-RPM2/* to perl/* module name "RPM".
  - perl: install in vendor dir.
  ...

4.4.2 -> 4.4.3:
  ...
  - fix: revert to older/slower perldeps avoid getOutputFrom() error.
  - perl: handle the 'v' in "use v5.6.0" (#140597).
  - add run-time perl(...) provides name space dependency set.
  - resurrect a rpm-perl subpackage from perl-RPM2-0.66.
  ...
----------

Comment 11 Chris Weyl 2006-05-17 18:28:48 EDT
Didn't the author formerly work for RedHat, and write this while he worked
there?  Wouldn't RedHat have a record of this license?
Comment 12 Paul Howarth 2006-05-18 02:58:06 EDT
(In reply to comment #11)
> Didn't the author formerly work for RedHat, and write this while he worked
> there?  Wouldn't RedHat have a record of this license?

Or, in a similar vein, is the copyright of works produced by employees whilst
working for Red Hat assigned to Red Hat?
Comment 13 Paul Howarth 2006-08-10 13:25:25 EDT
Looks like this review is going nowhere, so I'll close it soon unless somebody
has something new to add.
Comment 14 Chris Weyl 2006-08-17 11:51:12 EDT
Isn't there anyone @redhat.com that can determine the original license of this
module?

Barring that, have we looked at RPM4 on CPAN?
Comment 15 Jose Pedro Oliveira 2006-09-21 08:34:17 EDT
CC += fedora-perl-devel-list@redhat.com

* I believe Jason is no longer around;  maybe Robin Norwood could take over
  this and try to contact the Red Hat Legal Department

* The core perl-RPM-Specfile package has the same problem:
   - same author (Chip Turner)
   - same initial packager (Chip Turner)
   - it doesn't contain any copyright information
     (http://search.cpan.org/dist/RPM-Specfile/)
   - the specfile states the license is "GPL or Artistic"
     $ rpm -qp --qf="%{license}\n" perl-RPM-Specfile-1.19-2.1.1.src.rpm
     GPL or Artistic

* The upstream RPM package started including the perl RPM[2] module as of 4.4.3.
  It now generates a rpm-perl subpackage (http://wraptastic.org/pub/rpm-4.4.x/)

  $ rpm -qpl rpm-perl-4.4.3-1.i386.rpm
  /usr/lib/perl5/site_perl/5.8.7/i386-linux-thread-multi/RPM2.pm
  /usr/lib/perl5/site_perl/5.8.7/i386-linux-thread-multi/auto/RPM2
  /usr/lib/perl5/site_perl/5.8.7/i386-linux-thread-multi/auto/RPM2/RPM2.so
  /usr/share/man/man3/RPM2.3pm.gz

  $ rpm -qpl rpm-perl-4.4.6-1.i386.rpm
  /usr/lib/perl5/site_perl/5.8.8/i386-linux-thread-multi/RPM.pm
  /usr/lib/perl5/site_perl/5.8.8/i386-linux-thread-multi/auto/RPM
  /usr/lib/perl5/site_perl/5.8.8/i386-linux-thread-multi/auto/RPM/RPM.so
  /usr/share/man/man3/RPM.3pm.gz

  Note: the rpm maintainer has renamed the perl module (RPM2 -> RPM)
Comment 16 Robin Norwood 2006-09-24 12:17:11 EDT
I can contact Chip to clarify the license.

Since FC5 and FC6 both still seem to have rpm-4.4.2, I assume we still want to
get this into extras, yes?
Comment 17 Chip Turner 2006-09-24 13:13:54 EDT
I wrote this and RPM::Specfile while at red hat.  The license is the same as
perl itself:


Perl5 is Copyright (C) 1993-2005, by Larry Wall and others.

It is free software; you can redistribute it and/or modify it under the terms of
either:

a) the GNU General Public License as published by the Free Software Foundation;
either version 1, or (at your option) any later version, or

b) the "Artistic License".

from: http://dev.perl.org/licenses/

A lawyer or two from RH contacted me and I've told them the above on a few
occasions.

I'll look into making new CPAN releases of these modules with a clear license as
well.

HTH
Comment 18 Robin Norwood 2006-09-24 18:20:28 EDT
And I didn't even have to say his name three times!

Paul, is the above sufficient, or do you need to wait for the new CPAN release?
Comment 19 Paul Howarth 2006-09-25 02:58:48 EDT
I'm happy to approve this now, but is Jason still around to maintain it?
Comment 20 Robin Norwood 2006-09-25 12:30:49 EDT
No, he isn't.  I'm maintaining most of the perl* packages for FC and RHEL, so I
could take it if no-one else wants it.  I'll send a quick mail to
fedora-perl-devel to see if anyone else wants it first, though.
Comment 21 Robin Norwood 2006-11-22 13:18:04 EST
Turns out that there are/were a couple of existing bugs with this package.  I
haven't verified if they are still bugs with the latest version yet.

bugzilla #73921 - packaging issue
bugzilla #129724 - perl-RPM2 can't install multiple packages in a single
transaction.
Comment 22 Rahul Sundaram 2007-01-02 17:14:32 EST
What's the status on this review?
Comment 23 Paul Howarth 2007-01-03 06:21:55 EST
(In reply to comment #22)
> What's the status on this review?

The thing that was holding it up originally was the license. This has been fixed
by a new upstream release (0.67) which states that it's licensed under the same
terms as perl itself (i.e. GPL or Artistic).

In Comment #21, Robin was going to check whether Bug #73921 and Bug #129724 were
still present before going ahead with owning the package in Extras,

I have created an updated SRPM for 0.67 that brings the package much more into
line with the way perl modules are normally written for Extras. This addresses
the directory ownership issue of Bug #73921.

I haven't checked the status of Bug #129724 but that looks to me more like an
issue for upstream rather than a packaging issue, and I don't think that should
block the package from being imported.

Updated SRPM:
http://www.city-fan.org/~paul/extras/perl-RPM2/perl-RPM2-0.67-1.src.rpm
Updated Spec: http://www.city-fan.org/~paul/extras/perl-RPM2/perl-RPM2.spec

Robin, what's the state of play for this package from your point of view?
Comment 24 Robin Norwood 2007-01-03 12:24:55 EST
Paul, the new spec file looks great, and I agree that bug #129724 is a bug in
the upstream package.  My impression is that Chip won't be fixing it anytime
soon, so if we're ok with living with it, the package is fine as far as I'm
concerned.
Comment 25 Paul Howarth 2007-01-03 12:38:01 EST
(In reply to comment #24)
> Paul, the new spec file looks great, and I agree that bug #129724 is a bug in
> the upstream package.  My impression is that Chip won't be fixing it anytime
> soon, so if we're ok with living with it, the package is fine as far as I'm
> concerned.

Great stuff. I already approved the package earlier so feel free to import and
built it etc. (I assume you're still happy to own this package?)
Comment 26 Chris Weyl 2007-02-23 11:59:27 EST
ping? :)
Comment 27 Paul Howarth 2007-02-28 12:31:37 EST
Robin, are you going to own this package and import it?

Or does anyone else volunteer?
Comment 29 Robin Norwood 2007-03-01 10:48:47 EST
Sorry, dropped the ball on this one - I'll look into it today.
Comment 30 Robin Norwood 2007-03-01 11:21:23 EST
New Package CVS Request
=======================
Package Name: perl-RPM2
Short Description: Perl bindings for RPM
Owners: rnorwood@redhat.com
Branches: FC-5 FC-6
InitialCC: 
Comment 31 Jens Petersen 2007-03-01 20:20:38 EST
Added fedora-perl-devel-list@redhat.com in CC too.
Comment 32 Chris Weyl 2007-03-16 14:34:54 EDT
Was this branched to extras cvs?  I can't seem to find it in there.
Comment 33 Robin Norwood 2007-04-02 15:14:18 EDT
Yeah, i don't see it either - did I miss a step here?
Comment 34 Jens Petersen 2007-04-03 08:22:04 EDT
Sorry, seems I forgot to make the cvs module.
Should be there now, please try again.
Comment 35 Robin Norwood 2007-04-04 15:35:56 EDT
Hrm - checking out the module works for me now (cvs co perl-RPM2), but importing
it fails:

"""
[rnorwood@solitude fedora-extras]$ common/cvs-import.sh
/tmp/perl-RPM2-0.67-1.src.rpm 
Checking out module: 'perl-RPM2'
connect to address 10.8.34.151 port 544: Connection refused
cvs [checkout aborted]: end of file from server (consult above messages if any)
ERROR: "perl-RPM2" module does not exist in cvs.
[rnorwood@solitude fedora-extras]$ echo $CVSROOT
:ext:rnorwood@cvs.fedora.redhat.com:/cvs/extras
[rnorwood@solitude fedora-extras]$ echo $CVS_RSH
ssh
"""

Any ideas?
Comment 36 Jens Petersen 2007-04-04 20:04:43 EDT
(In reply to comment #35)
> connect to address 10.8.34.151 port 544: Connection refused
> cvs [checkout aborted]: end of file from server (consult above messages if any)

That doesn't look right.  Sorry but could you try again
and if you still have problems please ask on #fedora-admin or
cvsadmin-members at fedoraproject org.
Comment 37 Robin Norwood 2007-04-05 10:48:14 EDT
Ok, I figured out what was breaking.  The odd error message threw me off...I
added a -t to the cvs command in the cvs_import.sh script:

 -> Starting server: /usr/kerberos/bin/krsh -l rnorwood cvs.fedora.redhat.com
cvs server 


It's using krsh probably because I use krsh for my RH CVS checkout - my
~/.bashrc was setting CVS_RSH to it because of some changed I'd made recently.

Works now, thanks.
Comment 38 Robin Norwood 2007-04-05 11:26:16 EDT
Ok, ran builds for FC-5 and FC-6.  We'll see how they go.
Comment 39 Robin Norwood 2007-04-05 12:13:09 EDT
ok, looks like they both built successfully.  Finally done with this bug. :-)
Comment 40 Jose Pedro Oliveira 2007-05-06 11:28:48 EDT
(In reply to comment #39)
> ok, looks like they both built successfully.  Finally done with this bug. :-)

Not yet ;)
You haven't built it for development (Fedora 7).

jpo
Comment 41 Robin Norwood 2007-05-07 16:20:42 EDT
Oh.  I assumed f7 was going to include the newer version of rpm (your comment
#10), but it looks like we're at 4.4.2.

Anyway, built for f7, I'll send a request to rel-eng folks that it be included.
Comment 42 Robin Norwood 2007-05-08 10:22:42 EDT
"""
Package: perl-RPM2
NVR: perl-RPM2-0.67-2.fc7
User: jkeating
Status: complete
Tag Operation: tagged
Into Tag: f7-final

perl-RPM2-0.67-2.fc7 successfully tagged into f7-final by jkeating
"""

Ok, Jose, *now* can I close the bug?  ;-)

Comment 43 Robin Norwood 2008-08-25 12:36:14 EDT
Package Change Request
======================
Package Name: perl-RPM2
New Branches: EL-4 EL-5
Updated EPEL Owners: rnorwood
Comment 44 Kevin Fenzi 2008-08-25 15:53:41 EDT
cvs done.
Comment 45 Robin Norwood 2008-08-25 16:11:58 EDT
Thanks!
Comment 46 Lubomir Rintel 2008-12-11 19:41:21 EST
Package Change Request
======================
Package Name: perl-RPM2
New Branches: F-10

Note: This was declared dead due to incompatibility with RPM 4.6 before F-10 branching took place. It had been resurrected in CVS devel and patched for RPM 4.6 now.
Comment 47 Kevin Fenzi 2008-12-14 00:40:30 EST
cvs done.

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