Bug 1285515 (php-symfony-polyfill) - Review Request: php-symfony-polyfill - Symfony polyfills backporting features to lower PHP versions
Summary: Review Request: php-symfony-polyfill - Symfony polyfills backporting features...
Keywords:
Status: CLOSED ERRATA
Alias: php-symfony-polyfill
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: 1285514
Blocks: 1288785
TreeView+ depends on / blocked
 
Reported: 2015-11-25 19:42 UTC by Shawn Iwinski
Modified: 2015-12-29 03:53 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-12-19 18:27:40 UTC
fedora: fedora-review+


Attachments (Terms of Use)
phpci.log (13.48 KB, text/plain)
2015-12-07 06:25 UTC, Remi Collet
no flags Details
review.txt (8.35 KB, text/plain)
2015-12-07 06:32 UTC, Remi Collet
no flags Details

Description Shawn Iwinski 2015-11-25 19:42:07 UTC
Spec URL: https://raw.githubusercontent.com/siwinski/rpms/9d703acdc5cba592fbebab967065f0850e3107bc/php-symfony-polyfill/php-symfony-polyfill.spec

SRPM URL: https://siwinski.fedorapeople.org/SRPMS/php-symfony-polyfill-1.0.0-1.fc23.src.rpm

Description:
Symfony polyfills backporting features to lower PHP versions


Fedora Account System Username: siwinski

Comment 1 Upstream Release Monitoring 2015-12-02 18:19:52 UTC
siwinski's scratch build of php-symfony-polyfill-1.0.0-1.fc23.src.rpm for dist-6E-epel failed http://koji.fedoraproject.org/koji/taskinfo?taskID=12031564

Comment 2 Upstream Release Monitoring 2015-12-02 18:20:31 UTC
siwinski's scratch build of php-symfony-polyfill-1.0.0-1.fc23.src.rpm for f23 failed http://koji.fedoraproject.org/koji/taskinfo?taskID=12031559

Comment 3 Upstream Release Monitoring 2015-12-02 18:20:34 UTC
siwinski's scratch build of php-symfony-polyfill-1.0.0-1.fc23.src.rpm for epel7 failed http://koji.fedoraproject.org/koji/taskinfo?taskID=12031562

Comment 4 Upstream Release Monitoring 2015-12-02 18:21:00 UTC
siwinski's scratch build of php-symfony-polyfill-1.0.0-1.fc23.src.rpm for rawhide failed http://koji.fedoraproject.org/koji/taskinfo?taskID=12031558

Comment 5 Upstream Release Monitoring 2015-12-02 18:22:27 UTC
siwinski's scratch build of php-symfony-polyfill-1.0.0-1.fc23.src.rpm for f22 failed http://koji.fedoraproject.org/koji/taskinfo?taskID=12031560

Comment 6 Upstream Release Monitoring 2015-12-03 00:44:21 UTC
siwinski's scratch build of php-symfony-polyfill-1.0.0-2.fc23.src.rpm for rawhide failed http://koji.fedoraproject.org/koji/taskinfo?taskID=12033450

Comment 7 Remi Collet 2015-12-06 07:21:13 UTC
Damn.. I Haven't notice this review.

Again symfony does the stupid thing to merge everything in a single big repo... and then distribute various components from it...

Comment 8 Remi Collet 2015-12-06 07:47:28 UTC
See: https://github.com/symfony/polyfill/pull/15

Comment 9 Remi Collet 2015-12-06 07:50:32 UTC
I think it will be simpler to provide everything, whatever the PHP version is.

- this will be better with SCL.
- this will be noop (bootstrap will be ignored)
- this will avoid to have to add test in all consumer library

Ex: jeremeamia/superclosure requires symfony/polyfill-php56

See https://github.com/remicollet/remirepo/commit/5ac8cde9e3b7323bb01eaba76312529495296fe8

Comment 10 Upstream Release Monitoring 2015-12-06 07:53:12 UTC
remi's scratch build of php-symfony-polyfill-1.0.0-1.remi.src.rpm for rawhide completed http://koji.fedoraproject.org/koji/taskinfo?taskID=12080601

Comment 11 Shawn Iwinski 2015-12-06 14:50:28 UTC
I see 2 possible options:

1) Always provide ALL polyfills: https://github.com/siwinski/rpms/blob/e64463828a7c70097d7f7b8463983cc570b5f784/php-symfony-polyfill/php-symfony-polyfill-ALL.spec

    php-composer(ircmaxell/password-compat) and php-composer(paragonie/random_compat) will also always have to be available in this case as well.

2) Provide only needed polyfills and provide conditional MACROS to consumer libraries: https://github.com/siwinski/rpms/blob/e64463828a7c70097d7f7b8463983cc570b5f784/php-symfony-polyfill/php-symfony-polyfill-MACROS.spec

    Example consumer library usage:
        %if 0%{?php_symfony_polyfill_with_php56}
        Requires: php-composer(symfony/polyfill-php56)
        %endif



I much prefer option #2 because I'd prefer not to have so many no-ops, but I'm OK with option #1 if you believe it would be better.



(In reply to Remi Collet from comment #9)
> I think it will be simpler to provide everything, whatever the PHP version
> is.
> 
> - this will be better with SCL.

SCL only provides newer versions not older versions correct?


> - this will be noop (bootstrap will be ignored)

Ignored but still processed.


> - this will avoid to have to add test in all consumer library

See macros alternate above.


> Ex: jeremeamia/superclosure requires symfony/polyfill-php56
> 
> See
> https://github.com/remicollet/remirepo/commit/
> 5ac8cde9e3b7323bb01eaba76312529495296fe8

Comment 12 Remi Collet 2015-12-06 17:31:49 UTC
> SCL only provides newer versions not older versions correct?

In RHEL, yes.

But in Fedora, I think is make sens to be able to install older versions for compatibility test (ex dev with 5.6, test with 5.4 for deployment on RHEL-7)

Comment 14 Remi Collet 2015-12-07 06:25:09 UTC
Created attachment 1103024 [details]
phpci.log

phpCompatInfo version 4.5.2 DB built Nov 25 2015 18:15:42 CET static analyze results

Comment 15 Remi Collet 2015-12-07 06:32:46 UTC
Created attachment 1103030 [details]
review.txt

Generated by fedora-review 0.5.3 (bcf15e3) last change: 2015-05-04

Comment 16 Remi Collet 2015-12-07 06:36:27 UTC
[!]: Missing in %doc: Util/*md Util/composer.json
     and so, remove them from php dir.

[!]: Package must own all directories that it creates.
     Note: Directories without known owners: /usr/share/php/Symfony

     Must Own /usr/share/php/Symfony or require php-symfony-common


Notice, I'm ok not providing other compat layers (Iconv,Intl,Mbstring,Xml)
This only means if, from composer.json a app have 

Requires: php-composer(symfony/polyfill-intl)
Suggest:  php-intl

We'll have to use, which is fine :

Requires: php-intl

Please fix the 2 small blockers, everything else is ok.

Comment 17 Remi Collet 2015-12-07 06:48:38 UTC
Please also notice than https://github.com/symfony/polyfill/pull/15 is mandatory for upcomming PHPUnit 5 (I'm still blocked by PHPUnit_Selenium... but will happen...). Not mandatory for the review, we can wait for upstream merge, and apply it later, when needed.

Comment 18 Shawn Iwinski 2015-12-07 20:32:12 UTC
(In reply to Remi Collet from comment #16)
> [!]: Missing in %doc: Util/*md Util/composer.json
>      and so, remove them from php dir.
> 
> [!]: Package must own all directories that it creates.
>      Note: Directories without known owners: /usr/share/php/Symfony
> 
>      Must Own /usr/share/php/Symfony or require php-symfony-common
> 
> 
> Notice, I'm ok not providing other compat layers (Iconv,Intl,Mbstring,Xml)
> This only means if, from composer.json a app have 
> 
> Requires: php-composer(symfony/polyfill-intl)
> Suggest:  php-intl
> 
> We'll have to use, which is fine :
> 
> Requires: php-intl

Since RHEL's/Fedora's compiled versions of PHP include the extensions, I thought it best to require the use of the actual extension over the polyfill.



> Please fix the 2 small blockers, everything else is ok.

Fixed.  Diff = https://github.com/siwinski/rpms/commit/a25d03a83d872333a329640fd47f998026e5631c



(In reply to Remi Collet from comment #17)
> Please also notice than https://github.com/symfony/polyfill/pull/15 is
> mandatory for upcomming PHPUnit 5 (I'm still blocked by PHPUnit_Selenium...
> but will happen...). Not mandatory for the review, we can wait for upstream
> merge, and apply it later, when needed.

Based on your last comment on the pull request ("I confirm, this PR is no more required with PHPUnit 5.1.2.") I held off on adding the patch.





Spec URL: https://github.com/siwinski/rpms/blob/a25d03a83d872333a329640fd47f998026e5631c/php-symfony-polyfill/php-symfony-polyfill.spec

SRPM URL: https://siwinski.fedorapeople.org/SRPMS/php-symfony-polyfill-1.0.0-3.fc23.src.rpm

Comment 19 Remi Collet 2015-12-08 04:46:13 UTC
[x]: Missing in %doc: Util/*md Util/composer.json
[x]: Package must own all directories that it creates.



=== APPROVED ===

Comment 20 Gwyn Ciesla 2015-12-08 18:51:22 UTC
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/php-symfony-polyfill

Comment 21 Fedora Update System 2015-12-09 00:04:03 UTC
php-symfony-polyfill-1.0.0-3.el6 php-paragonie-random-compat-1.1.0-2.el6 has been submitted as an update to Fedora EPEL 6. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2015-b22c5c7367

Comment 22 Fedora Update System 2015-12-09 00:04:03 UTC
php-symfony-polyfill-1.0.0-3.fc22 php-paragonie-random-compat-1.1.0-2.fc22 has been submitted as an update to Fedora 22. https://bodhi.fedoraproject.org/updates/FEDORA-2015-6f7a7e13f2

Comment 23 Fedora Update System 2015-12-09 00:04:13 UTC
php-symfony-polyfill-1.0.0-3.el7 php-paragonie-random-compat-1.1.0-2.el7 has been submitted as an update to Fedora EPEL 7. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2015-f15e95a5bd

Comment 24 Fedora Update System 2015-12-09 00:04:13 UTC
php-symfony-polyfill-1.0.0-3.fc23 php-paragonie-random-compat-1.1.0-2.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2015-49570b2383

Comment 25 Fedora Update System 2015-12-09 05:20:01 UTC
php-paragonie-random-compat-1.1.0-2.el7, php-symfony-polyfill-1.0.0-3.el7 has been pushed to the Fedora EPEL 7 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=epel-testing update php-symfony-polyfill php-paragonie-random-compat'
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2015-f15e95a5bd

Comment 26 Fedora Update System 2015-12-09 05:49:34 UTC
php-paragonie-random-compat-1.1.0-2.el6, php-symfony-polyfill-1.0.0-3.el6 has been pushed to the Fedora EPEL 6 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=epel-testing update php-symfony-polyfill php-paragonie-random-compat'
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2015-b22c5c7367

Comment 27 Fedora Update System 2015-12-09 23:23:22 UTC
php-paragonie-random-compat-1.1.0-2.fc22, php-symfony-polyfill-1.0.0-3.fc22 has been pushed to the Fedora 22 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 'dnf --enablerepo=updates-testing update php-symfony-polyfill php-paragonie-random-compat'
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2015-6f7a7e13f2

Comment 28 Fedora Update System 2015-12-10 04:55:57 UTC
php-paragonie-random-compat-1.1.0-2.fc23, php-symfony-polyfill-1.0.0-3.fc23 has been pushed to the Fedora 23 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 'dnf --enablerepo=updates-testing update php-symfony-polyfill php-paragonie-random-compat'
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2015-49570b2383

Comment 29 Fedora Update System 2015-12-19 18:27:35 UTC
php-paragonie-random-compat-1.1.0-2.fc23, php-symfony-polyfill-1.0.0-3.fc23 has been pushed to the Fedora 23 stable repository. If problems still persist, please make note of it in this bug report.

Comment 30 Fedora Update System 2015-12-20 00:23:36 UTC
php-paragonie-random-compat-1.1.0-2.fc22, php-symfony-polyfill-1.0.0-3.fc22 has been pushed to the Fedora 22 stable repository. If problems still persist, please make note of it in this bug report.

Comment 31 Fedora Update System 2015-12-28 21:26:12 UTC
php-paragonie-random-compat-1.1.0-2.el7, php-symfony-polyfill-1.0.0-3.el7 has been pushed to the Fedora EPEL 7 stable repository. If problems still persist, please make note of it in this bug report.

Comment 32 Fedora Update System 2015-12-29 03:53:39 UTC
php-paragonie-random-compat-1.1.0-2.el6, php-symfony-polyfill-1.0.0-3.el6 has been pushed to the Fedora EPEL 6 stable repository. If problems still persist, please make note of it in this bug report.


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