Skip to content

Conversation

@itafroma
Copy link

Composer unconditionally "autoloads" files using require (see composer/composer#3003): this causes problems with globally-installed dependencies or where the composer autoload happens to be called more than once (see guzzle/guzzle#676).

One solution is to guard against function redeclaration using function_exists(), something that both guzzle/promises and React do (see L4-7 in functions.php and reactphp/promise#25, respectively).

The solution guzzle/promises currently uses does not work: PHP will attempt to load the functions before the conditional is evaluated. I've opted for the React solution, which is to point to a shim functions_include.php file that checks for the existence of the first function within functions.php before including functions.php.

@itafroma
Copy link
Author

Mostly-identical PRs for guzzle/guzzle and guzzle/promises were filed:

@mtdowling
Copy link
Member

I'd much rather this not be a new included file. It could just be a conditional at the top of the file and a return.

@itafroma
Copy link
Author

@mtdowling That won't work: functions are interpreted before the conditional. This will return "hi!" for example:

print foo();

function foo() {
    print "hi!";
}

This will return a fatal:

function foo() {}

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

function foo() {
    print "hi!";
}

@trowski
Copy link

trowski commented Jun 25, 2015

The method below does work and keeps things in a single file.

function foo() {
    return "hi!";
}

if (!function_exists('foo')) {
    function foo() {
        return "goodbye!";
    }
}

echo foo(), "\n"; // Echoes hi!

So just wrap the function definitions in functions.php with if (!function_exists(__NAMESPACE__ . '\str')) { ... }.

@itafroma
Copy link
Author

itafroma commented Jul 1, 2015

@mtdowling saw you merged the corresponding PRs for guzzle/promises and guzzle/guzzle: thanks! Is this PR good to go, or do you want to handle this one differently?

@mtdowling
Copy link
Member

Yeah, this PR is fine. Thanks.

mtdowling added a commit that referenced this pull request Jul 1, 2015
Conditionally require functions.php
@mtdowling mtdowling merged commit 0d374e0 into guzzle:master Jul 1, 2015
@magnetik
Copy link

Is there any plan to release a stable version with this fix anytime soon?

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants