Bug 1131731

quick notes (before review)

In description:
WARNING: %{_datadir}/php/Pimple.php is only provided ...
Should be %{_datadir}/php/Pimple/Pimple.php (which is really provided)

This compat file should probably have: include __DIR__.'/Container.php';
for project which use old version without autoloader.

As discussed earlier (for guzzle), see my comment about the autoloader and PSR-0/PSR-4 order issue for local build.

   echo "n" | make test

Cleaner to use:

   export NO_INTERACTION=1
   make test

Perhaps (need to be checked), also need 

   export REPORT_EXIT_STATUS=1

As for twig, I think having the minimal load test "outside" the %if %{with_tests} seems better.

Using "make test" (for ZTS) will imply to wait for PHP 5.5.16 push in updates.

I confirm, you must use

   make test NO_INTERACTION=1 REPORT_EXIT_STATUS=1

Else failure will not break the build.

Changes: https://github.com/siwinski/rpms/commit/bcd7f36e479c48c7e8c09c96be2eeb8d81c99f50



Spec URL: https://raw.githubusercontent.com/siwinski/rpms/bcd7f36e479c48c7e8c09c96be2eeb8d81c99f50/php-pimple.spec

SRPM URL: https://siwinski.fedorapeople.org/SRPMS/php-pimple-3.0.0-2.fc20.src.rpm

Created attachment 930739 [details]
phpci.log

phpCompatInfo version 3.3.0

Created attachment 930740 [details]
review.txt

Generated by fedora-review 0.5.1 (bb9bf27) last change: 2013-12-13

[!]: Requires correct, justified where necessary.

php-reflection detected by phpcompatinfo
Ok, this one is always present, but, to be consistent, 
as you BR it, you should also require it. (minor)

[!]: Package functions as described.

Pimple/Pimple.php compatibility class is unusable

Without the C extension, the include must come before the class_alias, else:

    Warning: Class 'Pimple\Container' not found in /usr/share/php/Pimple/Pimple.php on line 10


"BUT", with the C extension

    Fatal error: Cannot redeclare class Pimple\Container in /usr/share/php/Pimple/Container.php on line 34

Indeed, Pimple\Container is provided by the C extension, the .php file is only for compatibility when the C extension not available.

And making the include conditional, like:

    if (!extension_loaded("pimple")) {
        include __DIR__ . '/Container.php';
    }
    class_alias('Pimple\Container', 'Pimple');

Doesn't work...

    Warning: First argument of class_alias() must be a name of user defined class in /usr/share/php/Pimple/Pimple.php on line 13


This means:
- the PHP library is not needed with the C extension
- the compatibility cannot be provided (so probably php-pimple should only obsolete php-Pimple in rawhide)


Any other solution welcome...
For now, I don't really have the "right" answer to this issue.

Trying a separation of extension and library:


Spec URL: https://raw.githubusercontent.com/siwinski/rpms/cb229217977ebc8850670294da545ede26676e5f/php-pimple.spec

SRPM URL: https://siwinski.fedorapeople.org/SRPMS/php-pimple-3.0.0-3.fc20.src.rpm

Added autoloader



Spec URL: https://raw.githubusercontent.com/siwinski/rpms/48ead0fcb0a9664d967084706004e83782cc7ac6/php-pimple/php-pimple.spec

SRPM URL: https://siwinski.fedorapeople.org/SRPMS/php-pimple-3.0.0-5.fc22.src.rpm

We are definitively late for this one... sorry :/

Can you please update to 3.0.1 ?

The issue (comment #6) was only related to the compat stuff.

As this is no more provided (and we have php-pimple1 in the repo) it seems much better.

I think you can drop the conflicts.
Both can be installed.

If consumer use some autoloader (the one provided, of any other one), it will never be used, as all the classes and interfaces are provided by the extension.

Of course a direct include of the class won't work, but this is expected.

And so, you can suggest to use the extension (in the library package) to improve speed.

Updated.

Changes: https://github.com/siwinski/rpms/commit/b500ae872015867bb764a6805227c5ef29e31290



Spec URL: https://raw.githubusercontent.com/siwinski/rpms/b500ae872015867bb764a6805227c5ef29e31290/php-pimple/php-pimple.spec

SRPM URL: https://siwinski.fedorapeople.org/SRPMS/php-pimple-3.0.1-1.fc22.src.rpm

Created attachment 1058584 [details]
phpci.log

Created attachment 1058585 [details]
review.txt

Everything is OK now.
No blocker


=== APPROVED ===

THANKS for the review!



New Package SCM Request
=======================
Package Name: php-pimple
Short Description: A simple dependency injection container for PHP
Upstream URL: http://pimple.sensiolabs.org/
Owners: siwinski
Branches: f21 f22 epel7
InitialCC:

New Package SCM Request
=======================
Package Name: php-pimple
Short Description: A simple dependency injection container for PHP
Upstream URL: http://pimple.sensiolabs.org/
Owners: siwinski
Branches: f21 f22 f23 epel7
InitialCC:

Git done (by process-git-requests).

php-pimple-3.0.1-1.fc21 has been submitted as an update for Fedora 21.
https://admin.fedoraproject.org/updates/php-pimple-3.0.1-1.fc21

php-pimple-3.0.1-1.fc22 has been submitted as an update for Fedora 22.
https://admin.fedoraproject.org/updates/php-pimple-3.0.1-1.fc22

php-pimple-3.0.1-1.el7 has been submitted as an update for Fedora EPEL 7.
https://admin.fedoraproject.org/updates/php-pimple-3.0.1-1.el7

php-pimple-3.0.1-1.fc23 has been submitted as an update for Fedora 23.
https://admin.fedoraproject.org/updates/php-pimple-3.0.1-1.fc23

php-pimple-3.0.1-1.fc23 has been pushed to the Fedora 23 stable repository.

php-pimple-3.0.1-1.fc22 has been pushed to the Fedora 22 stable repository.

php-pimple-3.0.1-1.fc21 has been pushed to the Fedora 21 stable repository.

php-pimple-3.0.1-1.el7 has been pushed to the Fedora EPEL 7 stable repository. If problems still persist, please make note of it in this bug report.