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

[4.x]: php craft plugin/install --all aborts plugin install for any exception #13208

Closed
khalwat opened this issue May 16, 2023 · 3 comments
Closed

Comments

@khalwat
Copy link
Contributor

khalwat commented May 16, 2023

What happened?

Description

If you run php craft plugin/install --all as part of your project setup script, it will abort the installation of any plugin that throws an exception during the install process:

        try {
            $success = Craft::$app->getPlugins()->installPlugin($handle);
        } catch (Throwable $e) {
            $success = false;
        } finally {
            if (!$success) {
                $this->stderr("*** failed to install $handle" . (isset($e) ? ": {$e->getMessage()}" : '.') . PHP_EOL . PHP_EOL, Console::FG_RED);
                return ExitCode::UNSPECIFIED_ERROR;
            }
        }

https://github.com/craftcms/cms/blob/develop/src/console/controllers/PluginController.php#L280

While this might seem reasonable, there are soft errors (such as being unable to register a Twig extension) that can happen as part of the install process.

For example, when SEOmatic is installed, it ends up rendering Twig as part of the install process, as discussed here:

nystudio107/craft-seomatic#1312

Normally this is fine, because either Twig is already initialized, or it's part of a single CLI install of SEOmatic, so harm if Twig then gets initialized.

However as part of a bulk install via php craft plugin/install --all there are other plugins that get installed after it, and thus fail to install via the CLI.

What I propose is that instead of just considering all exceptions to mean that the plugin couldn't be installed, make an exception for the \LogicException exception that is thrown by Twig:

    public function addExtension(ExtensionInterface $extension): void
    {
        $class = \get_class($extension);

        if ($this->initialized) {
            throw new \LogicException(sprintf('Unable to register extension "%s" as extensions have already been initialized.', $class));
        }

        if (isset($this->extensions[$class])) {
            throw new \LogicException(sprintf('Unable to register extension "%s" as it is already registered.', $class));
        }

        $this->extensions[$class] = $extension;
    }

https://github.com/twigphp/Twig/blob/3.x/src/ExtensionSet.php#L127

...and treat that as a "soft" exception that is safe to complete the installation of the plugin.

Whilst I realize this seems like asking you to work around a quirk of SEOmatic, I've gone through many iterations of how I could potentially fix or special-case this, and none are particularly satisfying.

It's also in some ways a state introduced by the php craft plugin/install --all command that did not exist prior, in that plugins are installed one after the other in the same request, something that never happened prior.

I'm open to ideas on other ways to affect a fix as well.

Steps to reproduce

  1. Have SEOmatic and another plugin that installs after SEOmatic (alphabetically) that installs a Twig extension in your composer.json
  2. Do php craft plugin/install --all

Expected behavior

Plugins would all install

Actual behavior

Plugins that register a Twig extension after SEOmatic fail to install.

Craft CMS version

4.4.x

PHP version

8.1

Operating system and version

n/a

Database type and version

n/a

Image driver and version

n/a

Installed plugins and versions

SEOmatic 4.0.24

@brandonkelly
Copy link
Member

This should be fixed for the next release.

@brandonkelly
Copy link
Member

Craft 4.4.12 is out now with that fix.

@khalwat
Copy link
Contributor Author

khalwat commented May 25, 2023

Woo, thanks!

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

2 participants