Bug 837668 - Review Request: php-doctrine-DoctrineDBAL - Doctrine Database Abstraction Layer
Summary: Review Request: php-doctrine-DoctrineDBAL - Doctrine Database Abstraction Layer
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Remi Collet
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 517641 823043 837666
Blocks: 837669
TreeView+ depends on / blocked
 
Reported: 2012-07-04 16:20 UTC by Shawn Iwinski
Modified: 2013-06-29 18:25 UTC (History)
4 users (show)

Fixed In Version: php-doctrine-DoctrineDBAL-2.3.4-2.fc19
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2013-06-24 03:23:48 UTC
fedora: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)
phpci.log (49.37 KB, text/plain)
2013-06-10 11:43 UTC, Remi Collet
no flags Details
review.txt (7.28 KB, text/plain)
2013-06-10 11:44 UTC, Remi Collet
no flags Details

Description Shawn Iwinski 2012-07-04 16:20:28 UTC
Spec URL: http://siwinski.fedorapeople.org/rpmbuild/SPECS/php-doctrine-DoctrineDBAL.spec

SRPM URL: http://siwinski.fedorapeople.org/rpmbuild/SRPMS/php-doctrine-DoctrineDBAL-2.2.2-1.fc17.src.rpm

Description:
Powerful database abstraction layer with many features for database schema
introspection, schema management and PDO abstraction.

Fedora Account System Username: siwinski

Comment 1 Shawn Iwinski 2012-07-04 16:39:40 UTC
rpmlint outputs the following:

php-doctrine-DoctrineDBAL.noarch: E: script-without-shebang /usr/bin/doctrine-dbal.php
php-doctrine-DoctrineDBAL.noarch: E: zero-length /usr/share/doc/pear/DoctrineDBAL/Doctrine/DBAL/README.markdown
php-doctrine-DoctrineDBAL.noarch: E: incorrect-fsf-address /usr/share/doc/pear/DoctrineDBAL/LICENSE
php-doctrine-DoctrineDBAL.noarch: W: no-manual-page-for-binary doctrine-dbal.php
php-doctrine-DoctrineDBAL.noarch: W: no-manual-page-for-binary doctrine-dbal

* I will fix script-without-shebang according to http://fedoraproject.org/wiki/Common_Rpmlint_issues#script-without-shebang

* Since this is a PEAR package and all files are listed in package.xml, are zero-length errors required to be fixed?

* incorrect-fsf-address: There is only whitespace and mailing address differences.  I will work with upstream to get this fixed.  Is this a blocker until it is fixed upstream?  http://fedoraproject.org/wiki/Common_Rpmlint_issues#incorrect-fsf-address is not clear if this is a blocker or not.

* I will ask upstream to fix no-manual-page-for-binary warnings

Comment 2 Shawn Iwinski 2012-07-07 02:11:43 UTC
(In reply to comment #1)
> * I will fix script-without-shebang according to
> http://fedoraproject.org/wiki/Common_Rpmlint_issues#script-without-shebang

If I remove the execute mode bit from /usr/bin/doctrine-dbal.php, rpmlint gives the warning "non-executable-in-bin /usr/bin/doctrine-dbal.php 0644L".  How should this file be handled?

> * incorrect-fsf-address: There is only whitespace and mailing address
> differences.  I will work with upstream to get this fixed.  Is this a
> blocker until it is fixed upstream? 
> http://fedoraproject.org/wiki/Common_Rpmlint_issues#incorrect-fsf-address is
> not clear if this is a blocker or not.

DoctrineDBAL (actually all Doctrine projects) is changing from LGPL license to MIT license in future release.

See:
* https://groups.google.com/d/topic/doctrine-dev/1TfuNPiRvks/discussion
* http://dlm.beberlei.de/licenses/projects
* https://github.com/doctrine/dbal/commit/b3c08a7f0a1b0d8defe01448992a491481866dfc

Comment 3 Remi Collet 2012-07-07 04:35:59 UTC
(In reply to comment #2)
> (In reply to comment #1)
> > * I will fix script-without-shebang according to
> > http://fedoraproject.org/wiki/Common_Rpmlint_issues#script-without-shebang
> 
> If I remove the execute mode bit from /usr/bin/doctrine-dbal.php, rpmlint
> gives the warning "non-executable-in-bin /usr/bin/doctrine-dbal.php 0644L". 
> How should this file be handled?

The "source" file should include the shebang, which is

#!@php_bin@

And the package.xml should include a replacement order

<file ...
  <tasks:replace from="@php_bin@" to="php_bin" type="pear-config" />
</file>

This could be done by patching the upstream archive

A common usage is to drop the suffix for such command under linux only. You can look at "phpci" which use such solution (and also rename / ignore for .bat which is windows specific)
https://github.com/remicollet/php-compat-info/blob/master/scripts/phpci
https://github.com/remicollet/php-compat-info/blob/master/package.xml#L583
https://github.com/remicollet/php-compat-info/blob/master/package.xml#L1208

Of course, this should be proposed to upstream

> > * incorrect-fsf-address: There is only whitespace and mailing address
> > differences.  I will work with upstream to get this fixed.  Is this a
> > blocker until it is fixed upstream? 
> > http://fedoraproject.org/wiki/Common_Rpmlint_issues#incorrect-fsf-address is
> > not clear if this is a blocker or not.

This is not a blocker, You must only "report" this upstream.

> DoctrineDBAL (actually all Doctrine projects) is changing from LGPL license
> to MIT license in future release.

So, I think, it's ok to ignore this "temporary" warning.

Comment 4 Shawn Iwinski 2012-11-28 01:59:29 UTC
- Updated to upstream version 2.3.0
- Added "%global pear_metadir" and usage in %install
- Added php-json require
- Updated %description
- PEAR package.xml fixes in %prep
- Changed RPM_BUILD_ROOT to %{buildroot}


Spec URL: http://siwinski.fedorapeople.org/rpmbuild/SPECS/php-doctrine-DoctrineDBAL.spec

SRPM URL: http://siwinski.fedorapeople.org/rpmbuild/SRPMS/php-doctrine-DoctrineDBAL-2.3.0-1.fc17.src.rpm

Comment 5 Remi Collet 2013-01-22 13:31:22 UTC
Can you please update to latest 2.3.2 before review ?
(will probably allow you to drop some hack from spec)

Comment 6 Remi Collet 2013-01-22 13:34:04 UTC
I don't understand why upstream want to install doctrine-dbal and doctrine-dbal.php in %{_bindir}... seems really ugly...

A single executable file, without suffix, and a correct shebang must be enough...

Comment 7 Shawn Iwinski 2013-06-07 17:08:53 UTC
(In reply to Remi Collet from comment #5)
> Can you please update to latest 2.3.2 before review ?
> (will probably allow you to drop some hack from spec)

Updated to 2.3.4


(In reply to Remi Collet from comment #6)
> I don't understand why upstream want to install doctrine-dbal and
> doctrine-dbal.php in %{_bindir}... seems really ugly...
> 
> A single executable file, without suffix, and a correct shebang must be
> enough...

Changes in RPM spec to create a single executable



Spec URL: http://siwinski.fedorapeople.org/rpmbuild/SPECS/php-doctrine-DoctrineDBAL.spec

SRPM URL: http://siwinski.fedorapeople.org/rpmbuild/SRPMS/php-doctrine-DoctrineDBAL-2.3.4-1.fc18.src.rpm

Comment 8 Remi Collet 2013-06-10 11:43:25 UTC
Created attachment 759161 [details]
phpci.log

phpcompatinfo version 2.17.0.

Comment 9 Remi Collet 2013-06-10 11:44:09 UTC
Created attachment 759162 [details]
review.txt

Generated by fedora-review 0.4.1 (b2e211f) last change: 2013-04-29
Buildroot used: fedora-rawhide-x86_64
Command line :/usr/bin/fedora-review -b 837668

Comment 10 Remi Collet 2013-06-10 11:44:55 UTC
Minor "should"

[!]: Requires correct, justified where necessary.
	According to package.xml, PEAR >= 1.6.1
	But as EPEL-5 not targeted, this is not a blocker.

[!]: Rpmlint is run on all rpms the build produces.
	README.markdown could be removed (if upstream keep it empty)
	E: zero-length /usr/share/doc/pear/DoctrineDBAL/Doctrine/DBAL/README.markdown


No blocker

=== APPROVED ===

Comment 11 Shawn Iwinski 2013-06-10 23:40:38 UTC
THANKS for the review!


New Package SCM Request
=======================
Package Name: php-doctrine-DoctrineDBAL
Short Description: Doctrine Database Abstraction Layer
Owners: siwinski
Branches: f18 f19 el6
InitialCC:

Comment 12 Gwyn Ciesla 2013-06-11 11:57:33 UTC
Git done (by process-git-requests).

Comment 13 Fedora Update System 2013-06-12 23:01:34 UTC
php-doctrine-DoctrineDBAL-2.3.4-2.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/php-doctrine-DoctrineDBAL-2.3.4-2.fc19

Comment 14 Fedora Update System 2013-06-12 23:01:54 UTC
php-doctrine-DoctrineDBAL-2.3.4-2.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/php-doctrine-DoctrineDBAL-2.3.4-2.fc18

Comment 15 Fedora Update System 2013-06-12 23:02:12 UTC
php-doctrine-DoctrineDBAL-2.3.4-2.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/php-doctrine-DoctrineDBAL-2.3.4-2.el6

Comment 16 Fedora Update System 2013-06-13 18:02:16 UTC
php-doctrine-DoctrineDBAL-2.3.4-2.fc19 has been pushed to the Fedora 19 testing repository.

Comment 17 Fedora Update System 2013-06-24 03:23:48 UTC
php-doctrine-DoctrineDBAL-2.3.4-2.fc18 has been pushed to the Fedora 18 stable repository.

Comment 18 Fedora Update System 2013-06-28 18:51:27 UTC
php-doctrine-DoctrineDBAL-2.3.4-2.el6 has been pushed to the Fedora EPEL 6 stable repository.

Comment 19 Fedora Update System 2013-06-29 18:25:44 UTC
php-doctrine-DoctrineDBAL-2.3.4-2.fc19 has been pushed to the Fedora 19 stable repository.


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