Composer install with --optimize-autoloader breaks WP frontend

[pierre-dargham]

Hello,

I try to use wp-cli 1.2.0 as a composer dependency (I want to have it at ./vendor/bin/wp, regardless of the server or environment).

After installing / updating composer dependencies with option --optimize-autoloader, it produces following fatal error in WP frontend :

Fatal error: Uncaught Error: Call to undefined function WP_CLI\Dispatcher\get_path() in /home/foo/www/project/vendor/wp-cli/wp-cli/php/WP_CLI/Runner.php on line 49

I see two majors problems :

1. Obviously, wp-cli bootstrap is not compatible with composer --optimize-autoloader option
2. It shows that some part of wp-cli is loaded in front, despite it is a command line tool

To me, second point is very annoying, it means that instability / bugs in wp-cli could break WP frontend. As it is a command line tool, no single file of wp-cli should be loaded in front. In fact, classic usage of composer auto-loading is meant to avoid this type of problems : as long as a class is not necessary, don't load it.

I see there already is an issue on related subject #4051, and I think we should rethink wp-cli composer auto-loading process, to get back to a more classical use of composer auto-loading. At current state, it is really blocking to use wp-cli as a composer dependency on big WordPress projects (that we usually deploy with a composer install --optimize-autoloader step).

I hope this bug report will help,
Bests regards,

Pierre

[schlessera]

Hello Pierre,

Thanks for the feedback.

We are indeed trying to work on making WP-CLI be a well-behaving Composer citizen. However, given that most of the existing codebase comes from a procedural approach, this is not without its difficulties. There are still many components within WP-CLI that simply cannot be auto-loaded at all, as they are not classes/interfaces/traits.

Can you share a bit more about your setup to help us pinpoint the current issues?

[kalinichenko88]

I have this issue too. I use own project structure looks like Bedrock (https://github.com/roots/bedrock)/

[danielbachhuber]

We should restore the pre-1.2.0 behavior here. It might be as simple as a kill switch in our bootstrap process that bails when the context isn't CLI.

[schlessera]

The parts that activate WP-CLI through Composer are the procedural files that get loaded through the Composer "files" autoloader directive. This is just part of dealing with procedural file in the autoloader.

So, basically, every command that is added through the "files" directive will at least call the WP_CLI::add_command() method.

We can however put a kill switch into WP_CLI::add_command() that will immediately bail when not in CLI.

[danielbachhuber]

So, basically, every command that is added through the "files" directive will at least call the WP_CLI::add_command() method.

Ah, I see.

We can however put a kill switch into WP_CLI::add_command() that will immediately bail when not in CLI.

Yes, this seems like the most sensible approach to me (as hacky as it sounds).

[schlessera]

Okay, that kill-switch should prevent the errors on the front end. However, I noticed that we should change our "standard" command addition file.

Instead of if ( ! class_exists( 'WP_CLI' ) return; we should check for if ( !defined( 'WP_CLI' ) ) return;. The class exists on the front end as well if it is part of the same autoloader.

[danielbachhuber]

However, I noticed that we should change our "standard" command addition file.

Yes, although let's hold on doing that for now, because we might want to do something different in #3928

[danielbachhuber]

@pierre-dargham @kalinichenko88 Can you test and verify the fix in #4126 ?

[romaingugert]

Hello,

Using the "files" property from composer, is it really a good solution ?

Even if the bootstraping file to add the command to WPCli does nothing, PHP still executes the file.

Using your own autoloader when WPCli is initializing would not be better ?

For example, if you using composer as a dependency, it is possible to do an autoloader like this:

<?php
use Composer\DependencyResolver\Pool;
use Composer\Factory;
require_once __DIR__ .'/vendor/autoload.php';

// initialize composer
$composer = Factory::create(new Composer\IO\NullIO());

$repo = $composer->getRepositoryManager()->getLocalRepository();

foreach ($repo->getPackages() as $package) {
    if ($package->getType() === 'wp-cli-package') {
        // If extra key
        $extra = $package->getExtra();
        $file = isset($extra['register_command_file']) ? $extra['register_command_file'] : 'register_command.php';
        $filePath = $composer->getInstallationManager()->getInstallPath($package) . '/' .$file;
        if (file_exists($filePath)) {
            include $filePath;
        }
    }
}

Additionally, using the WP Cli commands as a suggestion rather than as a direct dependency on the project would allow developpers to select only the commands they need and thus lighten the WP Cli load.

Like consolidation/robo, it is possible to build the phar with the suggestion set.

[danielbachhuber]

Using the "files" property from composer, is it really a good solution ?

Not necessarily, but that issue is already tracked in #3928

The scope of this issue is to fix the unexpected breaking change introduced in 1.2.0

[pierre-dargham]

Thank you for the answers.

I just checked the fix in #4126 and it solves the issue : no more fatal error in WP frontend.

I agree it is probably the best solution for a short-term / 1.2.1 patch.

But in the long term, I would suggest a refactoring of the wp-cli bootstrap like suggested by @romaingugert, that seems a better solution than #3928 because it avoids to load multiple wp-cli command files during WP frontend bootstrap, and it solves the circular dependencies issue between wp-cli framework and wp-cli commands.

Bests regards,

Pierre

[danielbachhuber]

I just checked the fix in #4126 and it solves the issue : no more fatal error in WP frontend.

Glad to hear.

But in the long term, I would suggest a refactoring of the wp-cli bootstrap like suggested by @romaingugert,

We'll keep it in mind, thanks.

[kalinichenko88]

@danielbachhuber fix #4126 solves the issue. Thanks!