Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Autoloader loaded twice causing errors #91

Closed
lykciv opened this issue Jan 5, 2016 · 10 comments
Closed

Autoloader loaded twice causing errors #91

lykciv opened this issue Jan 5, 2016 · 10 comments

Comments

@lykciv
Copy link
Contributor

lykciv commented Jan 5, 2016

I'm currently working on a package within the vendor folder of a project. So, if I run grumphp, the following autoloaders are loaded:

  • /var/www/[project]/vendor/[company]/[package]/vendor/autoload.php
  • /var/www/[project]/vendor/autoload.php

This is causing errors when using the package beberlei/assert in both the package and the project.

The following fixes the error, but I noticed the break was removed in this commit (b23f63c).

// bin/grumphp
foreach ($autoloadLocations as $autoload) {
    if (is_file($autoload)) {
        require_once($autoload);
        $loaded = true;
        break;
    }
}

Full output of the error:

$ php ./vendor/bin/grumphp run
PHP Fatal error:  Cannot redeclare Assert\that() (previously declared in /var/www/[project]/vendor/[company]/[package]/vendor/beberlei/assert/lib/Assert/functions.php:36) in /var/www/[project]/vendor/beberlei/assert/lib/Assert/functions.php on line 39
PHP Stack trace:
PHP   1. {main}() /var/www/[project]/vendor/[company]/[package]/vendor/phpro/grumphp/bin/grumphp:0
PHP   2. require_once() /var/www/[project]/vendor/[company]/[package]/vendor/phpro/grumphp/bin/grumphp:16
PHP   3. ComposerAutoloaderInit6549f0c697930168ee67730da82614b8::getLoader() /var/www/[project]/vendor/autoload.php:7
PHP   4. composerRequire6549f0c697930168ee67730da82614b8($file = *uninitialized*) /var/www/[project]/vendor/composer/autoload_real.php:45

Fatal error: Cannot redeclare Assert\that() (previously declared in /var/www/[project]/vendor/[company]/[package]/vendor/beberlei/assert/lib/Assert/functions.php:36) in /var/www/[project]/vendor/beberlei/assert/lib/Assert/functions.php on line 39

Call Stack:
    0.0006     228320   1. {main}() /var/www/[project]/vendor/[company]/[package]/vendor/phpro/grumphp/bin/grumphp:0
    0.0083     586808   2. require_once('/var/www/[project]/vendor/autoload.php') /var/www/[project]/vendor/[company]/[package]/vendor/phpro/grumphp/bin/grumphp:16
    0.0087     598408   3. ComposerAutoloaderInit6549f0c697930168ee67730da82614b8::getLoader() /var/www/[project]/vendor/autoload.php:7
    0.0239    1199256   4. composerRequire6549f0c697930168ee67730da82614b8(???) /var/www/[project]/vendor/composer/autoload_real.php:45
@veewee
Copy link
Contributor

veewee commented Jan 5, 2016

The break was removed because both the autoloader from the project as the global autoloader should be loaded. When using normal classes, this isn't be a big problem. But because the assert library is registering functions, this is an issue. The only thing that can be done is adding function_exists() checks before defining the function. This needs to be done in the 3rd party package.

Maybe in your case it is better to install GrumPHP globally? Than it would use the autoloader of the current package + the global autoloader.

@veewee
Copy link
Contributor

veewee commented Jan 7, 2016

@lykciv Does the answer solve your issue? I don't think there is anything we can do about this.

@lykciv
Copy link
Contributor Author

lykciv commented Jan 11, 2016

Thank you for your feedback. I've installed grumphp globally, but unfortunately it doesn't solve my problem. The project autoloader is still used.

$autoloadLocations = array(
    getcwd() . '/vendor/autoload.php', // package autoloader
    getcwd() . '/../../autoload.php',  // project autoloader
    __DIR__ . '/../vendor/autoload.php', // does not exist
    __DIR__ . '/../../../autoload.php', // global autoloader
);

A possible solution would be to process the local and global autoloaders separately and do something like this:

$autoloadLocations = array(
    getcwd() . '/vendor/autoload.php',
    getcwd() . '/../../autoload.php',

);

$globalAutoloadLocations = array(
    __DIR__ . '/../vendor/autoload.php',
    __DIR__ . '/../../../autoload.php',
);

$loaded = false;
foreach ($autoloadLocations as $autoload) {
    if (is_file($autoload)) {
        require_once($autoload);
        $loaded = true;
        break;
    }
}

foreach ($globalAutoloadLocations as $autoload) {
    if (is_file($autoload)) {
        require_once($autoload);
        $loaded = true;
    }
}

@veewee
Copy link
Contributor

veewee commented Jan 12, 2016

That won't resolve the actual issue:
When you install assert both on the project and global, you will still receive the error.
There isn't anything we can really do about this issue.

One other option could be to add an environment variable like GRUNT_USE_AUTOLOADER to make it possible to specify one or more autoloader files.

@veewee veewee added the bug label Feb 1, 2016
@jenssegers
Copy link

Someone from our team had this issue as well, any news on this?

@veewee
Copy link
Contributor

veewee commented Feb 24, 2017

Currently there is no progress on this issue.

@syslogic
Copy link

the check should be for ! class_exists(), wrapped around the require_once() ... which would prevent duplicate require instructions, which subsequently result in duplicate auto-loading.

@veewee
Copy link
Contributor

veewee commented Jun 19, 2017

There is nothing wrong with duplicate autoloading. It is perfectly possible that you want to load both the globally installed packages + the framework specific packages. For classes this isn't a big issue since composer is smart enough to autoload them. Howerver, it is possible that 2 complete different versions of the same dependency are installed, which might cause errors.

The problem is with functions. Normally the package should check if the function already exists instead of just autoloading the functions. A good example on how to do this: https://github.com/symfony/polyfill-php71/blob/master/bootstrap.php

I would always recommend running grumphp in project mode and remove the globally installed version. It doesn't really add much value when it's installed globally in my opninion.

@syslogic
Copy link

it might depend on the case, if the 2 files are identical whether or not - because if both are the same version, one can already skip on file access. most granular might be an option to prefer global / local classes, if such a use case should exist.

@veewee
Copy link
Contributor

veewee commented Aug 30, 2019

Since the path system got a rewrite in #644 which has better autolaoding support, I am closing this issue for now.
Feel free to test that PR out and validate if it still works on your environment.
A new release will follow soon-ish.
Feel free to open up a new issue if this PR doesn't solve this issue for you.

@veewee veewee closed this as completed Aug 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants