Bug 1225134

Summary: Review Request: composer - Dependency Manager for PHP
Product: [Fedora] Fedora Reporter: Remi Collet <fedora>
Component: Package ReviewAssignee: Shawn Iwinski <shawn>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: package-review, shawn
Target Milestone: ---Flags: shawn: fedora-review+
gwync: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: composer-1.0.0-0.5.20150620gitd0ff016.el7 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2015-07-03 18:33:36 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 Flags
fedora-review-build-2015-06-20.log
none
phpcompatinfo-all.log
none
phpcompatinfo-src.log
none
fedora-review.txt none

Description Remi Collet 2015-05-26 16:27:16 UTC
Spec URL: https://raw.githubusercontent.com/remicollet/remirepo/7e86cbbaca398381ff70279f4e51f82ce658a79b/composer/composer.spec
SRPM URL: http://rpms.famillecollet.com/SRPMS/composer-1.0.0-0.5.20150525git69210d5.remi.src.rpm
Description: 
Composer helps you declare, manage and install dependencies of PHP projects,
ensuring you have the right stack everywhere.

Documentation: https://getcomposer.org/doc/


Fedora Account System Username: remi

-- 

This package is not about changing packaging of PHP libraries / applications, but only about providing the composer command.

Comment 1 Shawn Iwinski 2015-05-26 17:41:00 UTC
Perhaps "/usr/share/composer/res" could change to "/usr/share/composer/COMPOSER_VENDOR/COMPOSER_PROJECT/res" and then we could reuse "/usr/share/composer" for all packages to put their non-license, non-doc, and non-lib files in?

Since all files are RPM packaged and versioned, don't we (or shouldn't we) need to remove \Composer\Util\SpdxLicensesUpdater::update()?

Nice... I didn't see the removal of the self update command in the spec but I notice upstream now only loads that command if using Composer as a PHAR :)

Comment 2 Remi Collet 2015-05-26 17:59:12 UTC
(In reply to Shawn Iwinski from comment #1)
> Perhaps "/usr/share/composer/res" could change to
> "/usr/share/composer/COMPOSER_VENDOR/COMPOSER_PROJECT/res" and then we could
> reuse "/usr/share/composer" for all packages to put their non-license,
> non-doc, and non-lib files in?

Each package can own its /usr/share/<package> directory...
I don't see the benefit of this proposal... (as explain in description, I don't plan to change packaging Guildelines, just want to offer the "composer" command)

> Since all files are RPM packaged and versioned, don't we (or shouldn't we)
> need to remove \Composer\Util\SpdxLicensesUpdater::update()?

This is "only" dead code, as we don't provide the  update-spdx-licenses command

> Nice... I didn't see the removal of the self update command in the spec but
> I notice upstream now only loads that command if using Composer as a PHAR :)

;)

Comment 3 Shawn Iwinski 2015-05-26 18:09:43 UTC
(In reply to Remi Collet from comment #2)
> (In reply to Shawn Iwinski from comment #1)
> > Perhaps "/usr/share/composer/res" could change to
> > "/usr/share/composer/COMPOSER_VENDOR/COMPOSER_PROJECT/res" and then we could
> > reuse "/usr/share/composer" for all packages to put their non-license,
> > non-doc, and non-lib files in?
> 
> Each package can own its /usr/share/<package> directory...
> I don't see the benefit of this proposal... (as explain in description, I
> don't plan to change packaging Guildelines, just want to offer the
> "composer" command)

I thought at one point you mentioned it would be nice to have a location to put composer.json files other than /usr/share/doc/PKG/.  This is what I was thinking with this comment.

Comment 4 Remi Collet 2015-05-27 05:21:32 UTC
(In reply to Shawn Iwinski from comment #3)
> I thought at one point you mentioned it would be nice to have a location to
> put composer.json files other than /usr/share/doc/PKG/.  This is what I was
> thinking with this comment.

Yes, but such change will imply that all packages dropping a file in this directory will have to require "composer" for directory ownership, when it is really unneeded at runtime.

Comment 6 Shawn Iwinski 2015-06-21 03:53:11 UTC
Created attachment 1041338 [details]
fedora-review-build-2015-06-20.log

Comment 7 Shawn Iwinski 2015-06-21 03:54:13 UTC
See attachment fedora-review-build-2015-06-20.log from comment #6 for fedora-review build failure.

Comment 8 Remi Collet 2015-06-21 05:55:34 UTC
Damn test which need internet...

I open https://github.com/composer/composer/pull/4169
And so: http://koji.fedoraproject.org/koji/taskinfo?taskID=10172691

Comment 10 Shawn Iwinski 2015-06-21 17:24:34 UTC
Created attachment 1041463 [details]
phpcompatinfo-all.log

phpCompatInfo version 4.2.0 DB built May 22 2015 19:29:18 CEST

Comment 11 Shawn Iwinski 2015-06-21 17:25:40 UTC
Created attachment 1041464 [details]
phpcompatinfo-src.log

phpCompatInfo version 4.2.0 DB built May 22 2015 19:29:18 CEST

Comment 12 Shawn Iwinski 2015-06-21 17:50:35 UTC
Created attachment 1041490 [details]
fedora-review.txt

Generated by fedora-review 0.6.0 (3c5c9d7) last change: 2015-05-20
Command line :/usr/bin/fedora-review -m fedora-rawhide-x86_64 -b 1225134
Buildroot used: fedora-rawhide-x86_64
Active plugins: Generic, Shell-api
Disabled plugins: Java, C/C++, Python, fonts, SugarActivity, Ocaml, Perl, Haskell, R, PHP, Ruby
Disabled flags: EXARCH, DISTTAG, EPEL5, BATCH, EPEL6

Comment 13 Shawn Iwinski 2015-06-21 17:55:56 UTC
[!]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
     Note: rm -rf %{buildroot} present but not required
[!]: Each %files section contains %defattr if rpm < 4.4
     Note: %defattr present but not needed
[!]: Buildroot is not present
     Note: Buildroot: present but not needed
[!]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
     Note: %clean present but not required

As usual, remove EPEL5 bits after initial import.



The phpcompatinfo regular requires seem to be a mix of build requires and regular requires while the build requires seem very sparse.  Also, the regular requires is missing php-dom.  I'll let you choose the details there though.



No blockers.



===== APPROVED =====

Comment 14 Remi Collet 2015-06-21 18:00:33 UTC
Thanks for this review !

New Package SCM Request
=======================
Package Name: composer
Short Description: Dependency Manager for PHP
Upstream URL: https://getcomposer.org/
Owners: remi
Branches: f21 f22 epel7
InitialCC:

Comment 15 Gwyn Ciesla 2015-06-22 14:03:44 UTC
Git done (by process-git-requests).

Comment 16 Fedora Update System 2015-06-22 15:44:38 UTC
composer-1.0.0-0.5.20150620gitd0ff016.fc21 has been submitted as an update for Fedora 21.
https://admin.fedoraproject.org/updates/composer-1.0.0-0.5.20150620gitd0ff016.fc21

Comment 17 Fedora Update System 2015-06-22 15:44:45 UTC
composer-1.0.0-0.5.20150620gitd0ff016.fc22 has been submitted as an update for Fedora 22.
https://admin.fedoraproject.org/updates/composer-1.0.0-0.5.20150620gitd0ff016.fc22

Comment 18 Fedora Update System 2015-06-22 15:44:52 UTC
composer-1.0.0-0.5.20150620gitd0ff016.el7 has been submitted as an update for Fedora EPEL 7.
https://admin.fedoraproject.org/updates/composer-1.0.0-0.5.20150620gitd0ff016.el7

Comment 19 Shawn Iwinski 2015-06-22 19:38:38 UTC
@remi I forgot to mention, with Symfony 2.7 coming soon, you'll want to update your autoloader to use "ClassLoader" instead of "UniversalClassLoader" ("deprecated since version 2.4, to be removed in 3.0. Use the ClassLoader class instead").

Comment 20 Fedora Update System 2015-06-24 15:58:09 UTC
composer-1.0.0-0.5.20150620gitd0ff016.fc22 has been pushed to the Fedora 22 testing repository.

Comment 21 Fedora Update System 2015-07-03 18:33:36 UTC
composer-1.0.0-0.5.20150620gitd0ff016.fc22 has been pushed to the Fedora 22 stable repository.

Comment 22 Fedora Update System 2015-07-03 18:39:16 UTC
composer-1.0.0-0.5.20150620gitd0ff016.fc21 has been pushed to the Fedora 21 stable repository.

Comment 23 Fedora Update System 2015-07-14 16:47:47 UTC
composer-1.0.0-0.5.20150620gitd0ff016.el7 has been pushed to the Fedora EPEL 7 stable repository.