Bug 957573
| Summary: | Review Request: php-aws-sdk - amazon web services sdk for php | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Joseph Marrero <jmarrero> | ||||||
| Component: | Package Review | Assignee: | Gregor Tätzner <gregor> | ||||||
| Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> | ||||||
| Severity: | medium | Docs Contact: | |||||||
| Priority: | medium | ||||||||
| Version: | rawhide | CC: | fedora, gregor, jmarrero, notting | ||||||
| Target Milestone: | --- | Flags: | gregor:
fedora-review+
|
||||||
| Target Release: | --- | ||||||||
| Hardware: | All | ||||||||
| OS: | Linux | ||||||||
| Whiteboard: | |||||||||
| Fixed In Version: | php-aws-sdk-2.4.5-2.fc19 | Doc Type: | Bug Fix | ||||||
| Doc Text: | Story Points: | --- | |||||||
| Clone Of: | Environment: | ||||||||
| Last Closed: | 2013-09-22 23:55:57 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: | |||||||||
| Attachments: |
|
||||||||
|
Description
Joseph Marrero
2013-04-29 04:03:46 UTC
successful koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=5311290 PRE-REVIEW
- please rename channel.xml to %{name}.xml and adjust the scriptlets accordingly
"License: Apache-v2"
- you just made that up, did you ;)? Use 'ASL 2.0'
- all folders located in %{pear_phpdir}/AWSSDKforPHP/ beginning with underscore should be moved to %{pear_docdir} I don't know why just a couple of html files are marked as doc in the package.xml.
- also this should be fixed:
php-aws-sdk.noarch: E: non-executable-script /usr/share/pear/AWSSDKforPHP/_compatibility_test/sdk_compatibility_test_cli.php 0644L /usr/bin/env
php-aws-sdk.noarch: E: non-executable-script /usr/share/pear/AWSSDKforPHP/_samples/cli-s3_progress_bar.php 0644L /usr/bin/env
- watch your %doc macro
- "Source0: Source0: http://pear.amazonwebservices.com/get/sdk-%{version}.tgz" leads to unexpected results ;)
oh, and the description should be more verbose the package.xml is renamed to {pear_name}.xml
%{name}.xml is used when is a pear channel package. Even the template provided with rpmdev-newspec -t php-pear php-pear-foo uses pear_name macro
-licence FIXED (sorry, I should have fixed that before submiting)
- Documentation moved to the correct folder ({pear_docdir}/%{pear_name})
-%doc macro FIXED(removed doubled $doc)
Source0 Leads to unexpected results?
http://pear.amazonwebservices.com/get/sdk-1.6.2.tgz
-Description FIXED
How do you get??? (rpmlint and install don't complain) " php-aws-sdk.noarch: E: non-executable-script /usr/share/pear/AWSSDKforPHP/_compatibility_test/sdk_compatibility_test_cli.php 0644L /usr/bin/env
php-aws-sdk.noarch: E: non-executable-script /usr/share/pear/AWSSDKforPHP/_samples/cli-s3_progress_bar.php 0644L /usr/bin/env "
Spec URL: http://jmarrero.fedorapeople.org/packages/php-aws-sdk/php-aws-sdk.spec
SRPM URL: http://jmarrero.fedorapeople.org/packages/php-aws-sdk/php-aws-sdk-1.6.2-2.fc19.src.rpm
old packages and specs at: http://jmarrero.fedorapeople.org/packages/php-aws-sdk/old/
(In reply to comment #4) > the package.xml is renamed to {pear_name}.xml > %{name}.xml is used when is a pear channel package. Even the template > provided with rpmdev-newspec -t php-pear php-pear-foo uses pear_name macro the template from rpmdev is just a recommendation. Our guidelines are mandatory. At the end of the day 'sdk.xml' is just too short. http://fedoraproject.org/wiki/Packaging:PHP#PEAR_Modules > Source0 Leads to unexpected results? may I draw your attention on the double source tag: 'Source0: Source0: ...' :) > -Description FIXED thanks > How do you get??? (rpmlint and install don't complain) " > php-aws-sdk.noarch: E: non-executable-script > /usr/share/pear/AWSSDKforPHP/_compatibility_test/sdk_compatibility_test_cli. > php 0644L /usr/bin/env > php-aws-sdk.noarch: E: non-executable-script > /usr/share/pear/AWSSDKforPHP/_samples/cli-s3_progress_bar.php 0644L > /usr/bin/env " just a regular rpmlint run on f18. those script should be executable (In reply to comment #4) > How do you get??? (rpmlint and install don't complain) " > php-aws-sdk.noarch: E: non-executable-script > /usr/share/pear/AWSSDKforPHP/_compatibility_test/sdk_compatibility_test_cli. > php 0644L /usr/bin/env > php-aws-sdk.noarch: E: non-executable-script > /usr/share/pear/AWSSDKforPHP/_samples/cli-s3_progress_bar.php 0644L > /usr/bin/env " though on a second build I couldn't reproduce this issues. probably we're fine. and please clear the empty doc folder in '/usr/share/pear/AWSSDKforPHP/_docs' should be all fixed now: Spec URL: http://jmarrero.fedorapeople.org/packages/php-aws-sdk/php-aws-sdk.spec SRPM URL: http://jmarrero.fedorapeople.org/packages/php-aws-sdk/php-aws-sdk-1.6.2-3.fc19.src.rpm old packages and specs at: http://jmarrero.fedorapeople.org/packages/php-aws-sdk/old/ Created attachment 742106 [details]
pre-review
issues left:
- insufficient requires; see output of 'phpci print --recursive --report extension ...AWSSDKforPHP'
- the tarball ships multiple licenses. I spotted ASL and BSD. Please identify them, adjust the license tag and leave a comment about the location of the files
Spec URL: http://jmarrero.fedorapeople.org/packages/php-aws-sdk/php-aws-sdk.spec SRPM URL: http://jmarrero.fedorapeople.org/packages/php-aws-sdk/php-aws-sdk-1.6.2-4.fc19.src.rpm old packages and specs at: http://jmarrero.fedorapeople.org/packages/php-aws-sdk/old/ Created attachment 742915 [details]
diff 1.6.2-3 1.6.2.4
allright, thanks for the quick response
APPROVED
New Package SCM Request ======================= Package Name: php-aws-sdk Short Description: Amazon web services sdk for php Owners: jmarrero Branches: f17 f18 f19 el6 InitialCC: Git done (by process-git-requests). php-aws-sdk-1.6.2-4.fc18 has been submitted as an update for Fedora 18. https://admin.fedoraproject.org/updates/php-aws-sdk-1.6.2-4.fc18 php-aws-sdk-1.6.2-4.el6 has been submitted as an update for Fedora EPEL 6. https://admin.fedoraproject.org/updates/php-aws-sdk-1.6.2-4.el6 php-aws-sdk-1.6.2-4.fc17 has been submitted as an update for Fedora 17. https://admin.fedoraproject.org/updates/php-aws-sdk-1.6.2-4.fc17 php-aws-sdk-1.6.2-4.fc19 has been submitted as an update for Fedora 19. https://admin.fedoraproject.org/updates/php-aws-sdk-1.6.2-4.fc19 php-aws-sdk-1.6.2-4.fc17 has been pushed to the Fedora 17 testing repository. Sorry to get here a bit late. But I really thinkg this package shouldn't have been approved.
1/ a lot of file are docs.
Better solution to move docs in docs, is to change "role" in the package.xml, this allow to keep pear metadata consistent (check "pear list aws/sdk")
So, for example:
sed -e '/_samples/s/role="php"/role="doc"/' \
-e '/_docs/s/role="php"/role="doc"/' \
-e '/_compatibility_test/s/role="php"/role="doc"/' \
-e '/_sql/s/role="php"/role="doc"/' \
-e '/LICENSE/s/role="php"/role="doc"/' \
-e '/README/s/role="php"/role="doc"/' \
package.xml >%{pear_name}-%{version}/%{name}.xml
Of course, this is an upstream bug which should be reported and fixed upstream.
2/ bundled libraries
- AWSSDKforPHP/lib/cachecore
- AWSSDKforPHP/lib/dom
- AWSSDKforPHP/lib/yaml
- AWSSDKforPHP/lib/requestcore
Guidelines prohibit this.
https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries
At least symfony/yaml is available in pear channel so upstream so use a pear dependency.
And this will solves the Licensing issue.
3/ cacert.pem
I think this shouldn't be package, isn't the system one enough (ca-certificates) ?
This package can't even be installed... as it requires 2 opcode cache (xcache and apc), and, by design, only one can be installed. I think this should be a admin option (not a mandatory requires) As this are used for user data cache (not for opcode cache), I think requiring a user data only cache will be better (APCU will be a dropin replacement for APC, when approved, as APC is going to be removed from repo). Please also check phpci output ------------------------------------------------------------------------------- PHP COMPAT INFO EXTENSION SUMMARY ------------------------------------------------------------------------------- EXTENSION EXT min/Max PHP min/Max COUNT ------------------------------------------------------------------------------- Core 4.0.0 4.0.0 3019 PDO 5.1.0 5 Reflection 5.0.0 2 SPL 5.1.0 5.0.0 97 SimpleXML 5.0.0 22 XCache 1.0 4.3.0 4 apc 3.0.13 4.3.0 4 ctype 4.0.4 2 curl 4.0.2 257 date 4.0.0 59 dom 5.0.0 13 hash 1.1 4.0.0 13 json 5.2.0 5.2.0 49 libxml 5.1.0 5.0.0 2 mbstring 4.0.6 2 memcache 0.2 4.3.11 3 memcached 0.1.0 5.2.0 2 openssl 4.0.4 7 pcre 4.0.0 60 session 4.0.0 9 C sqlite3 0.7 5.3.0 1 standard 4.0.0 4.0.0 1690 zlib 1.0 4.0.0 16 ------------------------------------------------------------------------------- Per PHP Guidelines : "PHP extensions must have a Requires on all of the dependent extensions (php-date, php-gd, php-mbstring, ...). These extensions are virtual Provides of the php sub-packages." For example, as this library requires "dom", you must requires php-dom (not php-xml). Especially in Fedora >= 19, with php 5.5, there is a lot of change in extension/package split. thanks Remi, I've applied for commit access and will fix the remaining issues myself. thanks Remi, I unpushed the updates as soon as you posted the comments. Gregor, thanks I am in middle of finals I can not work on this until the 15th of May. Unsetting flag. * Wed May 08 2013 Gregor Tätzner <brummbq> - 1.6.2-5 - unbundle sfyaml - fix requires - mark doc in package.xml (In reply to comment #19) > 2/ bundled libraries yaml is out. these 3 are left: > - AWSSDKforPHP/lib/cachecore > - AWSSDKforPHP/lib/dom > - AWSSDKforPHP/lib/requestcore Do you want to create review requests? > 3/ cacert.pem > > I think this shouldn't be package, isn't the system one enough > (ca-certificates) ? probably, but we should handle this in our hypothetical php-requestcore package. php-aws-sdk-2.4.5-2.fc20 has been submitted as an update for Fedora 20. https://admin.fedoraproject.org/updates/php-aws-sdk-2.4.5-2.fc20 Package php-aws-sdk-2.4.5-2.fc20: * should fix your issue, * was pushed to the Fedora 20 testing repository, * should be available at your local mirror within two days. Update it with: # su -c 'yum update --enablerepo=updates-testing php-aws-sdk-2.4.5-2.fc20' as soon as you are able to. Please go to the following url: https://admin.fedoraproject.org/updates/FEDORA-2013-16275/php-aws-sdk-2.4.5-2.fc20 then log in and leave karma (feedback). php-aws-sdk-2.4.5-2.fc19 has been submitted as an update for Fedora 19. https://admin.fedoraproject.org/updates/php-aws-sdk-2.4.5-2.fc19 php-aws-sdk-2.4.5-2.fc20 has been pushed to the Fedora 20 stable repository. If problems still persist, please make note of it in this bug report. php-aws-sdk-2.4.5-2.fc19 has been pushed to the Fedora 19 stable repository. If problems still persist, please make note of it in this bug report. |