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

Latest commit breaks installer because of declaration conflict #5

Open
ddnetters opened this issue Feb 14, 2024 · 1 comment
Open

Comments

@ddnetters
Copy link

ddnetters commented Feb 14, 2024

The latest commit aimed at fixing migrations breaks the installer: 3e8b370. When installing the plugin, the following exception gets thrown:

*** installing promptly
PHP Compile Error 'yii\base\ErrorException' with message 'Cannot declare class MostlySerious\Promptly\Migrations\Install, because the name is already in use'

in /var/www/html/vendor/mostlyserious/craft-promptly/src/migrations/Install.php:0

Stack trace:
#0 [internal function]: yii\base\ErrorHandler->handleFatalError()
#1 {main}

This happens because CraftCMS calls requires_once on the default Install migration of a plugin (https://github.com/craftcms/cms/blob/develop/src/base/Plugin.php#L317). But because the Plugin class of promptly already declares the class by requiring it, the exception gets thrown. It goes through the following flow:

  1. A user runs php craft plugin/install promptly
  2. Craft boots
  3. Craft loads the Plugin class of promptly
  4. The class requires MostlySerious\Promptly\Migrations\Install
  5. Craft class Plugin::install()
  6. In the call stack the function runs require_once "/var/www/html/vendor/mostlyserious/craft-promptly/src/migrations/Install.php"; (https://github.com/craftcms/cms/blob/develop/src/base/Plugin.php#L317)
  7. PHP compiler crashes because the class was already declared

That's also why the exception only happens on a fresh install.

A quick solution would be to just extract all code in MostlySerious\Promptly\Migrations\Install::safeUp(); to some static helper class so you can call that in your Plugin.php and Install doesn't get declared again.

@corneliusio
Copy link
Collaborator

@ddnetters Thanks for calling this out. Craft's way of handling install migrations is basically made to not make sense. I've reverted this change and made the assumption that the only reason the table wasn't installing before is that the migrations directory was capitalized and Pixel & Tonic apparently hate PSR-4 standards. ¯_(ツ)_/¯

Let me know if you have any issue with the most recent update. This did install as expected for me on a completely fresh install so I have to believe it's fixed now, ha.

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

No branches or pull requests

2 participants