Cross pollination of deps between local packages can lead to function definition errors

[padraic]

Basically, using one composer enabled package locally upon another where both share hamcrest as a dependency leads to the following error where the function definitions clash. Solution is likely to check whether a function is defined before defining it.

PHP Fatal error:  Cannot redeclare assertThat() (previously declared in /home/padraic/projects/mockery/vendor/hamcrest/hamcrest-php/hamcrest/Hamcrest.php:22) in /home/padraic/projects/mockery/vendor/hamcrest/hamcrest-php/hamcrest/Hamcrest.php on line 29
PHP Stack trace:
PHP   1. {main}() -:0
PHP   2. Humbug\Env\Job::main() -:32
PHP   3. Humbug\Adapter\Phpunit::main() -:23
PHP   4. PHPUnit_TextUI_Command->run() /home/padraic/projects/humbug/src/Humbug/Adapter/Phpunit.php:165
PHP   5. PHPUnit_TextUI_Command->handleArguments() /home/padraic/projects/humbug/vendor/phpunit/phpunit/src/TextUI/Command.php:148
PHP   6. PHPUnit_TextUI_Command->handleBootstrap() /home/padraic/projects/humbug/vendor/phpunit/phpunit/src/TextUI/Command.php:651
PHP   7. PHPUnit_Util_Fileloader::checkAndLoad() /home/padraic/projects/humbug/vendor/phpunit/phpunit/src/TextUI/Command.php:817
PHP   8. PHPUnit_Util_Fileloader::load() /home/padraic/projects/humbug/vendor/phpunit/phpunit/src/Util/Fileloader.php:77
PHP   9. include_once() /home/padraic/projects/humbug/vendor/phpunit/phpunit/src/Util/Fileloader.php:93
PHP  10. require() /home/padraic/projects/mockery/tests/Bootstrap.php:80
PHP  11. ComposerAutoloaderInita674127f288ce71eed996dfd2cffaa8b::getLoader() /home/padraic/projects/mockery/vendor/autoload.php:7
PHP  12. composerRequirea674127f288ce71eed996dfd2cffaa8b() /home/padraic/projects/mockery/vendor/composer/autoload_real.php:49

[aik099]

Ideally we shouldn't include global function definitions by default in composer.json and instead move all functions into static class. That way assertThat becomes Hamcrest::assertThat. When Hamcrest namespace is imported as h then h::assertThat is even better.

That was original idea, when we moved PHP version of library to GitHub.

[aik099]

In #16 we've added global function to autoload because some people asked for it.

[padraic]

@aik099 Created a couple of PRs to both fix this and make phpunit runs a bit smoother.

[aik099]

It turns out that we created that static file with all matchers already. So doing use Hamcrest\Matchers as h; would do the trick.

[michael-donat]

This has been raised before I remember - 'Hamcrest\Matchers as hm' does the trick. I think the global functions file was left in for BC.

On 27 Dec 2014, at 09:14, Alexander Obuhovich notifications@github.com wrote:

It turns out that we created that static file with all matchers already. So doing use Hamcrest\Matchers as h; would do the trick.

— Reply to this email directly or view it on GitHub https://github.com/hamcrest/hamcrest-php/issues/18#issuecomment-68172216.

[aik099]

And that's why we're not including it (global functions file) by default through Composer. But now we do and we have some conflicts.

[sosso]

I believe this is breaking Mockery's test suite @Padraic

[aik099]

I see following solutions (we can either implement all or some of them, please vote):

1 . add following on top of hamcrest/Hamcrest.php generated file (yes return can be used in global context as well):

if ( function_exists('assertThat') ) {
    return;
}

2 . remove hamcrest/Hamcrest.php file autoloading from https://github.com/hamcrest/hamcrest-php/blob/master/composer.json#L12 and indicate in README.md, that people should use static file, but if they want they can do require_once 'vendor/hamcrest/hamcrest-php/hamcrest/Hamcrest.php'; to get global functions

[aik099]

@sosso , yes it does. @davedevelopment , haven't you noticed that Mockery test suite fails on Travis as well on latest PR merges?

[sosso]

I can't speak to existing deployments, but I think option one is the 'cleanest' solution?

[aik099]

Trick with return statement wasn't working because PHP was declaring these function even before that IF was called. I had to wrap all function declarations with a single large IF.

Please check if PR I've made (see above) works for you and then I'll merge it. Since people are getting fatal errors currently, then I think that making a bugfix release would be right thing to do after PR merge.

[davedevelopment]

Where is it breaking mockery's suite? The latest failures for Mockery are HHVM ones that have been there for some time.

[aik099]

There 1st include of Hamcrest.php with global function definitions in test suite bootstrap. Then 2nd include happens when vendor/autoload.php in included because composer.json of Hamcrest (maybe this Hamcrest version isn't released yet) always loads Hamcrest.php which causes that error: https://github.com/hamcrest/hamcrest-php/blob/master/composer.json#L12

[aik099]

Should be working now and I've made 1.2.1 release. Please test.

[sosso]

I was able to run the Mockery tests. Thanks!

On Tue, Jan 20, 2015 at 11:37 AM, Alexander Obuhovich < notifications@github.com> wrote:

Should be working now and I've made 1.2.1 release. Please test.

— Reply to this email directly or view it on GitHub https://github.com/hamcrest/hamcrest-php/issues/18#issuecomment-70717701 .