Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 1356552 (php-onelogin-php-saml)
Summary: | Review Request: php-onelogin-php-saml - SAML support for PHP softwares | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | James Hogarth <james.hogarth> | ||||||
Component: | Package Review | Assignee: | Remi Collet <fedora> | ||||||
Status: | CLOSED RAWHIDE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> | ||||||
Severity: | medium | Docs Contact: | |||||||
Priority: | medium | ||||||||
Version: | rawhide | CC: | fedora, package-review | ||||||
Target Milestone: | --- | Flags: | fedora:
fedora-review+
|
||||||
Target Release: | --- | ||||||||
Hardware: | All | ||||||||
OS: | Linux | ||||||||
Whiteboard: | |||||||||
Fixed In Version: | Doc Type: | If docs needed, set a value | |||||||
Doc Text: | Story Points: | --- | |||||||
Clone Of: | Environment: | ||||||||
Last Closed: | 2016-08-01 14:00:23 UTC | Type: | --- | ||||||
Regression: | --- | Mount Type: | --- | ||||||
Documentation: | --- | CRM: | |||||||
Verified Versions: | Category: | --- | |||||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||
Cloudforms Team: | --- | Target Upstream Version: | |||||||
Embargoed: | |||||||||
Bug Depends On: | 1356584 | ||||||||
Bug Blocks: | |||||||||
Attachments: |
|
Description
James Hogarth
2016-07-14 11:16:21 UTC
After discussion with siwiniski changing the requires to the proposed v1 library as despite it looking like it has a v2 requirement ... it really doesn't as that adds only adds namespacing that the embedded xmlseclibs explicitly removes. Marked as NotReady as it doesn't pass tests in rawhide, though it does in F24. Looks a difference in the assert() behaviour change from PHP7 is causing it, though the upstream defaults claim that they behave like PHP5. I'll remove the whiteboard marker when the issue is resolved. The Fedora PHP build has a zend.assertions that differs from the upstream default which is what is breaking the build/tests in Rawhide. Removing the NotReady now it's understood why this is the case. It can be built and tested on F24 in the meanwhile. Runtime code patched (and PR issued upstream) and after discussion with Remi phpunit being called with both development settings (so assert() related stuff gets tested once) and with system settings (to reflect actual use when assert() gets compiled out with production settings). Removing the blocker on the zend.assertions php ini setting as that's no longer relevant to this package. Note that the Rawhide build in koji for some reason: http://koji.fedoraproject.org/koji/taskinfo?taskID=14934573 It did however work in COPR: https://copr.fedorainfracloud.org/coprs/jhogarth/NextCloud/build/390252/ It also built correctly in local mock. The spec and srpm on #c1 is correct for testing. Built on a second try ... go figure ... koji must have had a bad moment: http://koji.fedoraproject.org/koji/taskinfo?taskID=14934630 Upstream has released a new version (2.9.1) with my PR merged. Spec URL: https://jhogarth.fedorapeople.org/php-saml/php-onelogin-php-saml.spec SRPM URL: https://jhogarth.fedorapeople.org/php-saml/php-onelogin-php-saml-2.9.1-1.fc25.src.rpm Created attachment 1182086 [details]
phpci.log
phpCompatInfo version 5.0.1 DB version 1.10.0 built Jul 05 2016 07:54:25 CEST static analyze results
From PHP Guidelines: "PHP extensions must have a Requires on all of the dependent extensions" From composer.json "require": { "php": ">=5.3.2", "ext-openssl": "*", "ext-dom": "*", "ext-mcrypt": "*" }, So please add Requires: php-openssl Requires: php-dom From phpcompatinfo analysis (attachement), also add Requires: php-date Requires: php-filter Requires: php-hash Requires: php-libxml Requires: php-pcre Requires: php-session Requires: php-zlib Suggests: php-gettext From PHP Guidelines: "A PSR-0 [1] compliant library would put its PHP files in /usr/share/php/<Vendor Name> " As provided classes are OneLogin_Saml_* and OneLogin_Saml2_* should be installed in (PSR-0 compliant tree): /usr/share/php/OneLogin/Saml /usr/share/php/OneLogin/Saml2 (autoloader in Saml or/and Saml2 directory) Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=14957389 Created attachment 1182090 [details]
review.txt
Generated by fedora-review 0.6.1 (f03e4e7) last change: 2016-05-02
Command line :/usr/bin/fedora-review -b 1356552
Buildroot used: fedora-rawhide-x86_64
Summary from review.txt [!]: Requires correct, justified where necessary. [!]: Package complies to the Packaging Guidelines [!]: Please add CHANGELOG as %doc Updated as per comments: In addition there with the separation of Saml and Saml2 there are two loaders in the Saml directory ... a compat one that uses static loading like that used in the phpunit tests and upstream's compatibility.php and also one using classname based loading such as carried out by the composer based loader in nextcloud's user_saml application. Spec URL: https://jhogarth.fedorapeople.org/php-saml/php-onelogin-php-saml.spec SRPM URL: https://jhogarth.fedorapeople.org/php-saml/php-onelogin-php-saml-2.9.1-2.fc25.src.rpm Koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=15012687 Following IRC discussion ... reduced to single autoloader but keeping split PSR directories: Spec URL: https://jhogarth.fedorapeople.org/php-saml/php-onelogin-php-saml.spec SRPM URL: https://jhogarth.fedorapeople.org/php-saml/php-onelogin-php-saml-2.9.1-3.fc25.src.rpm koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=15013330 --- php-onelogin-php-saml.spec.0 2016-07-20 13:37:25.000000000 +0200 +++ php-onelogin-php-saml.spec 2016-07-25 17:07:35.000000000 +0200 @@ -2,18 +2,22 @@ %global gh_short %(c=%{gh_commit}; echo ${c:0:7}) %global gh_owner onelogin %global gh_project php-saml +%global php_vendor OneLogin %global php_minver 5.3.2 Name: php-%{gh_owner}-%{gh_project} Version: 2.9.1 -Release: 1%{?dist} +Release: 3%{?dist} Summary: SAML support for PHP License: MIT URL: https://github.com/%{gh_owner}/%{gh_project} Source0: %{url}/archive/%{gh_commit}/%{gh_project}-%{version}-%{gh_short}.tar.gz +# Patch the test bootstrap for our autoload.php rather than adjust in %%check to simplify spec +Patch0: php-saml-bootstrap-autoloader.patch + BuildArch: noarch BuildRequires: php(language) >= %{php_minver} @@ -28,19 +32,36 @@ # From composer.json, "require": { # "php": ">=5.3.2" Requires: php(language) >= %{php_minver} +Requires: php-openssl +Requires: php-dom + # From manual unbundling, needs 1.4 contrary to the bundled 2.0 due to namespace issues Requires: php-composer(robrichards/xmlseclibs) >= 1.4.1 Requires: php-composer(robrichards/xmlseclibs) < 2.0.0 -Provides: php-composer(%{gh_owner}/%{gh_project}) = %{version} -# Uses the mcrypt algorithms +# From phpci analysis +Requires: php-date +Requires: php-filter +Requires: php-hash +Requires: php-libxml +Requires: php-pcre +Requires: php-session +Requires: php-zlib + + +Suggests: php-gettext + +# Uses the mcrypt algorithms which is a suggests in xmlseclibs Requires: php-mcrypt +Provides: php-composer(%{gh_owner}/%{gh_project}) = %{version} + + %description OneLogin's SAML PHP toolkit let you build a SP (Service Provider) over your PHP application and connect it to any IdP (Identity Provider). -Autoloader: %{_datadir}/php/%{gh_project}/autoload.php +Autoloader: %{_datadir}/php/%{php_vendor}/Saml2/autoload.php %prep @@ -50,27 +71,22 @@ %build rm -rf extlib : Generate autoloader -%{_bindir}/phpab -n --output lib/autoload.php lib +%{_bindir}/phpab -n --output lib/Saml2/autoload.php lib # Append the xmlseclibs requirement not in composer -cat >> lib/autoload.php <<EOF +cat >> lib/Saml2/autoload.php <<EOF require_once "%{_datadir}/php/robrichards-xmlseclibs/autoload.php"; EOF %install -mkdir -p %{buildroot}%{_datadir}/php/%{gh_project} -cp -pr lib/* %{buildroot}%{_datadir}/php/%{gh_project}/ +mkdir -p %{buildroot}%{_datadir}/php/%{php_vendor} +cp -pr lib/* %{buildroot}%{_datadir}/php/%{php_vendor}/ %check -: Run upstream tests -# Use our autoloader in tests -sed -i 's#_toolkit_loader.php#lib/autoload.php#' tests/bootstrap.php -# Drop the bundled xmlseclibs from being used in bootstrap -sed -i '/XMLSECLIBS_DIR/d' tests/bootstrap.php -: Run phpunit tests in dev mode +: Run upstream phpunit tests in dev mode %{_bindir}/php -c %{_docdir}/php/php.ini-development %{_bindir}/phpunit --verbose --debug --bootstrap tests/bootstrap.php --configuration tests/phpunit.xml -: Run phpunit tests in system settings mode +: Run upstream phpunit tests in system settings mode %{_bindir}/php %{_bindir}/phpunit --verbose --debug --bootstrap tests/bootstrap.php --configuration tests/phpunit.xml @@ -79,11 +95,17 @@ %files %license LICENSE -%doc advanced_settings_example.php settings_example.php README.md composer.json -%{_datadir}/php/%{gh_project} +%doc advanced_settings_example.php settings_example.php README.md composer.json CHANGELOG +%{_datadir}/php/%{php_vendor} %changelog +* Mon Jul 25 2016 James Hogarth <james.hogarth> - 2.9.1-3 +- Switch to a single autoloader after feedback + +* Mon Jul 25 2016 James Hogarth <james.hogarth> - 2.9.1-2 +- Update spec with comments from review + * Wed Jul 20 2016 James Hogarth <james.hogarth> - 2.9.1-1 - update to 2.9.1 [x]: Requires correct, justified where necessary. [x]: Package complies to the Packaging Guidelines [x]: Please add CHANGELOG as %doc Everything is now OK. It seems the "--debug" option is not needed (create lof ot output lines) === APPROVED === Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/php-onelogin-php-saml php-onelogin-php-saml-2.9.1-3.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2016-de1aca249f php-onelogin-php-saml-2.9.1-3.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2016-353d77a339 php-onelogin-php-saml-2.9.1-3.el7 has been submitted as an update to Fedora EPEL 7. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2016-81dff2c066 php-onelogin-php-saml-2.9.1-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. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2016-81dff2c066 php-onelogin-php-saml-2.9.1-3.fc23 has been pushed to the Fedora 23 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-de1aca249f php-onelogin-php-saml-2.9.1-3.fc24 has been pushed to the Fedora 24 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-353d77a339 php-onelogin-php-saml-2.9.1-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. php-onelogin-php-saml-2.9.1-3.fc24 has been pushed to the Fedora 24 stable repository. If problems still persist, please make note of it in this bug report. php-onelogin-php-saml-2.9.1-3.fc23 has been pushed to the Fedora 23 stable repository. If problems still persist, please make note of it in this bug report. |