Bug 517641 - Review Request: php-channel-doctrine - Adds doctrine project channel to PEAR
Summary: Review Request: php-channel-doctrine - Adds doctrine project channel to PEAR
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jason Tibbitts
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 517643 837666 837668 837669
TreeView+ depends on / blocked
 
Reported: 2009-08-15 14:16 UTC by Christof Damian
Modified: 2012-07-04 16:25 UTC (History)
3 users (show)

Fixed In Version: 1.0.0-2.fc11
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-08-27 02:16:10 UTC
tibbs: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Christof Damian 2009-08-15 14:16:33 UTC
Spec URL: http://rpms.damian.net/SPECS/php-channel-doctrine.spec
SRPM URL: http://rpms.damian.net/SRPMS/php-channel-doctrine-1.0.0-1.fc11.src.rpm
Description: 
This package adds the doctrine channel which allows
PEAR packages from this channel to be installed.

Comment 1 Christof Damian 2009-08-15 14:17:56 UTC
This is based on php-channel-symfony with s/symfony/doctrine/

Comment 2 Andrew Colin Kissa 2009-08-17 09:29:51 UTC
Since doctrine requires php >= 5.3.2 the requires should specify that

Comment 3 Christof Damian 2009-08-17 09:54:09 UTC
(In reply to comment #2)
> Since doctrine requires php >= 5.3.2 the requires should specify that  

Doctrine 1.1 requires 5.2.3, but this request is just for the doctrine pear channel and doesn't require a certain php version.

The bug 517643 is for the doctrine library and specifies 5.2.3 as required.

Comment 4 Andrew Colin Kissa 2009-08-17 10:01:02 UTC
I do not see the point in installing a channel which may not be usable, unless the channel provides any packages that do not require => php 5.2 3 (you do realise that the channel will not be used exclusively by your other package, nothing will stop a user from running pear install pear.phpdoctrine.org/Doctrine-1.0.x)

That information should be in the spec, such that anyone intending to use the spec for EPEL for instance will pick it up straight away that they cannot.

Comment 5 Christof Damian 2009-08-17 10:28:49 UTC
But the channel is usable by anyone, just not the Doctrine 1.1 package.

If any more packages get added to the doctrine channel in the future which don't require 5.2.3 I would have to change the minimum again, this doesn't make sense. Or once Doctrine 2.0 is released I would have to change it to 5.3, which is even worse.

But the main thing is: this channel doesn't require 5.2.3, Doctrine does.

Comment 6 Andrew Colin Kissa 2009-08-17 10:34:22 UTC
That's why am asking you if the channel provides other packages apart from doctrine if it does then fine, if it is just dedicated to doctrine then you have to enforce the checking.

Comment 7 Andrew Colin Kissa 2009-08-17 10:37:00 UTC
As far as i can tell this channel is dedicated only to doctrine

pear list-all -c pear.phpdoctrine.org
All packages [Channel pear.phpdoctrine.org]:
============================================
Package                       Latest Local
pear.phpdoctrine.org/Doctrine 1.1.3        PHP Doctrine ORM

Comment 8 Christof Damian 2009-08-17 10:41:40 UTC
At the moment it is. And at the moment it is mainly for Doctrine 1.1, but it is not going to stay this way. 

And it doesn't matter, because the channel doesn't require php 5.2.3.

Comment 9 Jason Tibbitts 2009-08-19 21:00:01 UTC
I have to agree with Christof here; the channel itself doesn't have any specific PHP version dependency, and it's foolish to try and track whatever might get added to the channel to somehow extract that information.  Besides, no supported Fedora release has anything older than php 5.2.6, so the guidelines indicate that we shouldn't have a versioned dependency in any case.

What troubles me is where you found an MIT license for the content in this package.  I would tend to lean towards the one small XML file in this package being non-copyrightable data, but then the fedora-bookmarks just has a list of URLs and it carries the GFDL license, so I guess it's good that I'm not a lawyer.  Doctrine itself is LGPLv2+.  I guess I'll block FE-Legal for an opinion, but you could just clear this up if you just ask upstream for some indication of the license of the channel file.  (Actually what really bothers me is that we have to be so anal about this, but that's the way it is.)

In any case, this is the epitome of a trivial package.  I'd approve it if the license issue were cleared up one way or the other.

* source file matches upstream.  sha256sum:                         
   215215f50b339b89d72b15cfa0273728dd2ba397c7d300c51a785f8223f4cdbc
   channel.xml
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.                                                              
* description is OK.                                                          
* dist tag is present.
* build root is OK.
? license field matches the actual license.
? license is open source-compatible.
* license text included in package.
* latest version is being packaged.
* BuildRequires are proper.
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly.
* rpmlint has acceptable complaints (no-documentation).
* final provides and requires are sane:
   php-channel(pear.doctrine-project.com)
   php-channel-doctrine = 1.0.0-1.fc12
  =
   /bin/sh
   /usr/bin/pear
   php-cli
   php-pear(PEAR)

* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no generically named files
* scriptlets are OK (pear channel registration)

Comment 10 Christof Damian 2009-08-20 13:42:20 UTC
I have contacted the doctrine people via twitter, mailing list and IRC which hopefully will clear the license question up.

Andrew pointed out in bug 517643 that Doctrine is a mix of LGPLv2+, MIT, and BSD. 

So, lets wait and see.

Comment 11 Christof Damian 2009-08-20 16:49:36 UTC
I got a response from jwage on twitter and he said it is LGPL. So here are the new files:

Spec URL: http://rpms.damian.net/SPECS/php-channel-doctrine.spec
SRPM URL:
http://rpms.damian.net/SRPMS/php-channel-doctrine-1.0.0-2.fc11.src.rpm

Comment 12 Jason Tibbitts 2009-08-20 17:14:05 UTC
Looks good; APPROVED.  I'll unblock FE-Legal.

By the way, your web server sends .spec files as application/octet-stream, so they can't be viewed in the browser.  A bit inconvenient, that.

Comment 13 Christof Damian 2009-08-20 17:46:50 UTC
New Package CVS Request
=======================
Package Name: php-channel-doctrine
Short Description: Adds doctrine project channel to PEAR
Owners: cdamian
Branches: F-11
InitialCC:

Comment 14 Jason Tibbitts 2009-08-20 17:58:14 UTC
CVS done.

Comment 15 Fedora Update System 2009-08-22 01:02:36 UTC
php-channel-doctrine-1.0.0-2.fc11 has been pushed to the Fedora 11 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update php-channel-doctrine'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F11/FEDORA-2009-8866

Comment 16 Fedora Update System 2009-08-27 02:16:04 UTC
php-channel-doctrine-1.0.0-2.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 17 Christof Damian 2010-07-04 11:18:38 UTC
Package Change Request
======================
Package Name: php-channel-doctrine
New Branches: EL-6
Owners: cdamian

Comment 18 Kevin Fenzi 2010-07-05 01:40:35 UTC
CVS done (by process-cvs-requests.py).


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