Composer support

[claylo]

As discussed in #522, this set of changes adds support for composer installation.

The package-composer.json file, coupled with package.xml and the package2composer script that is part of the Conductor package at http://pear.claylo.com/ is all that is needed to update the composer.json file.

Simply run package2composer in the top level of the checkout to regenerate composer.json to reflect changes made to package.xml

Suggestion: Link your repository at http://packagist.com. It's a one-time GitHub Service hookup, which will keep those using composer up to date as you produce releases using your normal process.

[Baachi]

👍 Awesome!
Thank you @claylo @sebastianbergmann

[peterhorne]

Thanks @claylo and @sebastianbergmann. I've been looking forward to this!

[michilehr]

Thank you guys for making this happen!

[sebastianbergmann]

@stof I'll gladly merge pull requests for the constraint issues you mention.

[toopay]

👍

[cordoval]

ya era hora!

[Seldaek]

Glad to see this moving forward. I tried it and found a few issues though, @claylo please take a look to double check: #647 #648 #649

[claylo]

@stof and @sebastianbergmann the upper-bound constraint issue is actually handled in the Package2XmlToComposer.php script.

https://github.com/claylo/conductor/blob/master/Conductor/Converter/Package2XmlToComposer.php#L548

If the PHPUnit* package.xml files begin supporting a max version value, subsequent runs of package2composer will pick those up. So, patch isn't necessary if PHPUnit team wants to start documenting max versions in their dependencies in package.xml.

@stof, regarding the extras section with branch aliasing, I will work out a way to support that -- probably by manually tweaking it in the package-composer.json config file.

[Gasol]

👍

[demonkoryu]

Yeeehaaa!

[Furizaa]

That is a very nice thing to happen 👍
But I get a lot of "Cannot redeclare" errors on the "Generating autoloader files" step. The worst thing is that every time it seems to be another one.

"require": {
    ...
    "phpunit/phpunit": "3.7.*"
}

Currently its Cannot redeclare php_tokenstream_autoload() (previously declared in /var/lib/jenkins/workspace/project/vendor/phpunit/php-token-stream/PHP/Token/Stream/Autoload.php:45) in /var/lib/jenkins/workspace/project/vendor/phpunit/php-token-stream/PHP/Token/Stream/Autoload.php on line 230

I did't want to pollute #649 with this.

[michilehr]

@Furizaa : run 'php composer.phar selfupdate', delete your vendor dir and run again 'php composer.phar install'

[claylo]

This is similar to giorgiosironi/phpunit-selenium#178

@Furizaa can you post your PHP version info? -- output of php --version? I'm curious if this is related to the Suhosin patch on OS X.

[Furizaa]

Got the same problem on two boxes:

My development VBox

PHP 5.3.10-1ubuntu3.2 with Suhosin-Patch (cli) (built: Jun 13 2012 17:19:58) 
Copyright (c) 1997-2012 The PHP Group
Zend Engine v2.3.0, Copyright (c) 1998-2012 Zend Technologies
    with Xdebug v2.2.1, Copyright (c) 2002-2012, by Derick Rethans

and the Jenkins Bare-Metal

PHP 5.3.10-1ubuntu3.4 with Suhosin-Patch (cli) (built: Sep 12 2012 18:59:41) 
Copyright (c) 1997-2012 The PHP Group
Zend Engine v2.3.0, Copyright (c) 1998-2012 Zend Technologies
    with Xdebug v2.1.0, Copyright (c) 2002-2010, by Derick Rethans
    with Suhosin v0.9.33, Copyright (c) 2007-2012, by SektionEins GmbH

[claylo]

@Seldaek Can you have a look at this? In my experiments, changing Composer's autoload_real.php file to use require_once instead of require causes this problem to disappear.

It's notable that the problem people are describing only presents itself in installations with the Suhosin Patch. I'm working with a friend who knows the PHP internals well to help me attempt to track down the issue within the Suhosin patch, if any.

I'm going to try various combinations of specifying only the include paths and the main PHPUnit Autoload.php (and none of the sub-package files, since PHPUnit calls those), but in the event that this comes up again with other projects, I'd like to suggest considering require_once instead of require. Blasphemous, I know, but performance hit these days is negligible compared to the old days.

[Furizaa]

Ok, @mickaelandrieu solution works on my dev box ... wich is awkward since my Jenkins build job fetches a fresh composer.phar from getcomposer.org for every build without vendors installed.

[claylo]

@Furizaa interesting. Can you post the version string of the composer self-update that's working for you now?

Also, you might want to consider installing Composer globally on your Jenkins box so you can skip that download.

[Furizaa]

So I learned today that apparently the version on getcomposer.org isn't that version you get by a self-update :)

vagrant@precise64:/var/www$ php composer.phar self-update
Updating to version 6bd7ca0.

I've added the self-update command to my Jenkins job that is also working now.
That Jenkins has to fetch composer for every build is ok. I want to simulate a new developer fetching the repository to start working.

[Seldaek]

@claylo I already tried to explain that the way autoloading is done in the phpunit packages is broken, it would certainly work better with classmap or psr0 if phpunit was patched to not depend on its own specific autoloader - patch which I also provided, though granted it might not be complete. Anyway this might indeed work better with one of the latest versions of composer that prevents multiple includes of vendor/autoload.php from triggering many autoloaders. If that doesn't fix it, then I guess it should be fixed in phpunit.

[claylo]

@Seldaek I understand that you and Sebastian don't agree on how Autoloading should be done. What I'm suggesting is that it's possible that others who want to leverage Composer won't agree with you either. That'll probably be rare, but could happen.

Composer supports specifying a classmap file, or other "custom" includes. That's the whole point of the "file" option, right? To support composer installation for projects that don't adhere to classmaps or PSR-0? It would be smoother if everyone did do it that way, but the existence of 'file' autoload support indicates composer acknowledges and "works" with scenarios where packages do not do classmaps and PSR-0.

Right now, the issue that @Furizaa describes is still a problem on the default OS X 10.8.1 PHP, which is 5.3.13 with Suhosin Patch 0.9.10.

In my test autoload_real.php file, I currently see:

require $vendorDir . '/phpunit/php-file-iterator/File/Iterator/Autoload.php';
require $vendorDir . '/phpunit/php-text-template/Text/Template/Autoload.php';
require $vendorDir . '/phpunit/php-token-stream/PHP/Token/Stream/Autoload.php';
require $vendorDir . '/phpunit/php-code-coverage/PHP/CodeCoverage/Autoload.php';
require $vendorDir . '/phpunit/php-timer/PHP/Timer/Autoload.php';
require $vendorDir . '/phpunit/phpunit-mock-objects/PHPUnit/Framework/MockObject/Autoload.php';
require $vendorDir . '/phpunit/phpunit/PHPUnit/Autoload.php';
require $vendorDir . '/phpunit/phpunit-selenium/PHPUnit/Extensions/SeleniumCommon/Autoload.php';

With Suhosin Patch-free PHP 5.3.15, this works just fine. With Suhosin Patched 5.3.13, though, it results in:

Fatal error: Cannot redeclare phpunit_selenium_autoload() (previously declared in /Users/clay/issue178-composer-phar/vendor/phpunit/phpunit-selenium/PHPUnit/Extensions/SeleniumCommon/Autoload.php:47) in /Users/clay/issue178-composer-phar/vendor/phpunit/phpunit-selenium/PHPUnit/Extensions/SeleniumCommon/Autoload.php on line 117

Using require_once OR rearranging the order of the files in this list fixes this problem. Reason: PHPUnit/Autoload.php does a require_once on PHPUnit/Extensions/SeleniumCommon/Autoload.php.

Since autoload_real.php follows with a require, the "cannot redeclare" error occurs.

PHPUnit aside, there are other scenarios where this sort of thing could happen. Why not bypass them all by using require_once in autoload_real.php, or at least provide a flag in the "File" autoload type that allows edge-case packages to request require_once be used in their own files?

TL;DR: I don't think it's reasonable to assume that PHPUnit is the only project that will ever run into this issue. If you really want to drive adoption of Composer, I don't think "fix your project" is a scalable solution.

[Furizaa]

@claylo I am able to reproduce this error. Hope this is helpful to you. OSX 10.8.0 with composer at HEAD.

➜  www-data git:(develop) php -v
PHP 5.3.13 with Suhosin-Patch (cli) (built: Jun 20 2012 17:05:20) 
Copyright (c) 1997-2012 The PHP Group
Zend Engine v2.3.0, Copyright (c) 1998-2012 Zend Technologies
➜  www-data git:(develop) php -i | grep Suhosin
This server is protected with the Suhosin Patch 0.9.10

[claylo]

thanks, @Furizaa. I've submitted pull requests on the PHPUnit family of packages that are already autoloaded BY PHPUnit. Once those changes are in place in those composer.json files, this issue should be resolved ... at least as far as PHPUnit is concerned.

[stof]

@claylo with the latest version of Composer, the require statement should never be called several time as the method will return early when the autoloader was already initialized once.

@Furizaa which url are you using to fetch composer on your jenkins box ? The version on getcomposer.org is the latest as this is where self-update is fetching it from

[claylo]

@stof the key word in your comment was "should" :)

What was happening is that PHPUnit's defined Autoload.php file was doing a require_once-if-present on optional dependencies, followed immediately by Composer doing a require on the same file.

I've fixed that in the case of the PHPUnit packages, but you won't know what others are going to do. Given that the computing overhead to use require_once is barely measurable compared to require, Composer is taking a stance on how things "should" be instead of supporting how things sometimes are.

A Google search on "tolerant on input, strict on output" brings up many, many additional voices suggesting that Composer's stance on require_once versus require is inverse of what it should be.

[Seldaek]

@claylo:

Composer supports specifying a classmap file, or other "custom" includes. That's the whole point of the "file" option, right? To support composer installation for projects that don't adhere to classmaps or PSR-0? It would be smoother if everyone did do it that way, but the existence of 'file' autoload support indicates composer acknowledges and "works" with scenarios where packages do not do classmaps and PSR-0.

• Composer does not support specifying a classmap, it can generate the classmap from the php files you point it to if you use the "classmap" autoloading option.
• "files" is mainly made to force-load functions since those can not be autoloaded. It shouldn't be abused since it means those files are required at every request, no matter if they're needed or not. Using it for a custom autoloader should not be needed since composer's autoloader can handle everything via either psr-0 or classmap.
• PHPUnit's problem, and why I said it should be fixed in PHPUnit, is two-fold: it has its own autoloader that automatically loads the autoloaders of other packages (meaning the package is not self-contained and that's not good) and it requires its own autoloader (or at least part of it) for some functionality, which could (should IMO) be achieved differently, see my other PR that was closed.

Your solution of removing the files autoloading from the other packages so that stuff isn't loaded twice is not a great idea IMO because it means those packages can not be used standalone anymore, since they don't have an autoload config for composer.

[claylo]

@Seldaek This is all a matter of opinion, and your personal preferences are clear. So are Sebastian's. I guess what it really boils down to is whether or not Composer wants to be broadly adopted and tolerant of use cases or scenarios that don't perfectly fit your personal world view.

FWIW, the files I changed for the packages I adjusted have PHPUnit as a dependency, so they weren't being used standalone anyway.

[Seldaek]

It's not just a preference, the current way of doing it is wasting cpu cycles. If you choose not to care that's fine, but for such a high profile project as PHPUnit I think this sort of attention to detail is warranted.

Anyway I'll drop the case, this isn't going anywhere useful.

[sebastianbergmann]

Yes, the way that the autoloaders are implemented violates Separation of Concerns (because the class map is also used to populate the blacklist that is used to filter stack traces and code coverage information). I do not like this either. And I want to change it in the future. But I can not, and will not, rush this. I have other things in my life to do than to make Composer happy.

People can bitch about the PEAR Installer all they want: for me it always "just worked". There are only two benefits of using Composer that I see so far:

Composer makes it eas{ier|y} to maintain multiple versions of the same package. The lack for this was my only gripe with the PEAR Installer. Why this feature has been implemented in a new tool instead of improving the old tool is not for me to judge (I do not know the implementation details of the PEAR Installer).

The second benefit that I see is that Composer makes it eas{ier|y} to use development versions of packages by instrumenting Git. While this is not something that I missed personally so far, I do see the value of it.

Besides these two aforementioned benefits, I only see problems.

[Seldaek]

@sebastianbergmann glad to see you agree that it's not an ideal way, no problem with not rushing it, as I pointed out in my PR I wasn't sure it's a complete solution, and I realize you can't do major changes without some BC assurance. Anyway the current workaround works so no problem, it's just not perfect, but it'll do for most people. It's not about making composer happy, I just tried to point out the problem because I noticed it.

As for PEAR vs Composer, I hope we can agree on "to each his own". That said I'd be happy to hear what problems you see, maybe not here though.