Sterc / seosuite

This repository contains the SEOSuite 2.x MODX extra.
GNU General Public License v2.0
13 stars 18 forks source link

Install of update fails - Cannot override frozen service "seosuite". #87

Closed JoshuaLuckers closed 1 year ago

JoshuaLuckers commented 1 year ago

When SEO Tools is installed you can't install an update.

Install failed with Pimple\Exception\FrozenServiceException in /core/vendor/pimple/pimple/src/Pimple/Container.php:85: Cannot override frozen service "seosuite".
Could not install package with signature: seosuite-3.1.1-rc7

I had to uninstall all prior versions of SEO Suite in order to install the update.

joeke commented 1 year ago

@JoshuaLuckers Yeah I've seen this too, I've fixed it by temporarily disabling the plugin before upgrade.. but I have the feeling it's maybe a MODX issue? Since SeoSuite has a plugin which loads in the service in the manager, but that is also loaded when trying to update it fails. Or is it something that should be fixed in a setup resolver by temporarily disabling the plugin? I'm not sure really, since then it also should re-enable the plugin when the update fails for some other reason.

bezumkin commented 1 year ago

@joeke I changed component bootstrap.php to check if SeoSuite already loaded

if (!$modx->services->has('seosuite')) {
    // Add your classes to modx's autoloader
    $modx->addPackage('Sterc\SeoSuite\Model', __DIR__ . '/src/', null, 'Sterc\\SeoSuite\\');

    // Register base class in the service container
    $modx->services->add('seosuite', function() use ($modx) {
        return new \Sterc\SeoSuite\SeoSuite($modx);
    });
}
JoshuaLuckers commented 1 year ago

I think it's because GPM includes this bootstrap resolver: https://github.com/Sterc/seosuite/blob/9ab62243f77e83ac236a88cb576ea4af69d65cd6/_build/gpm_resolvers/gpm.resolve.bootstrap.php#L26

This will load the bootstrap.php where it adds the seosuite service. But since it's already loaded it will fail.

JoshuaLuckers commented 1 year ago

@bezumkin that's the solution I was going to propose as well! You beat me to it.

joeke commented 1 year ago

@JoshuaLuckers @bezumkin Ah yes, I figured it would have something to do with the bootstrap file. I will include this fix in the next release. Thank you!

rthrash commented 1 year ago

Just chiming in on this that I first saw this when trying to upgrade from rc4 to rc7 … and just now with rc8. Keep up the great work team Sterc et al!

JoshuaLuckers commented 1 year ago

@joeke is there a timeline for the next release?

joeke commented 1 year ago

@JoshuaLuckers Probably somewhere end of this week.

joeke commented 1 year ago

Should be fixed now with release 3.1.2 https://modx.com/extras/package/seosuite.

JensWolff commented 1 year ago

Sadly I still get this error with 3.1.2 @joeke Installation failed with Pimple\Exception\FrozenServiceException in /core/vendor/pimple/pimple/src/Pimple/Container.php:85: Cannot override frozen service "seosuite". Could not install the package with signature "seosuite-3.1.2-pl".

joeke commented 1 year ago

@JensWolff Hm that's strange .. I've tested on a couple of installations but did not encounter the issue there. Then it must be something to do with the plugin I think. Could you maybe test it by first disabling the SEO Suite plugin, and then trying to update?

JensWolff commented 1 year ago

@joeke Tried it a second ago with deactivating the plugin first like described and this workaround definitely works. After the update, the plugin is also automatically active again.

joeke commented 1 year ago

@JensWolff Alright, thank you for confirming this. Then we'll have to dig into why the plugin would cause the FrozenServiceException error, since there's not much going in that. I'm guessing $modx->services->get('seosuite') in the plugin is most likely causing the problem. If I look into the core Services/Container class, and in the Pimple/Container class it looks like the services are all set on class construction .. @JoshuaLuckers @bezumkin Any guesses here?

JoshuaLuckers commented 1 year ago

@joeke Hmm, I'm not sure but I think it will only fix this issue once the update fixing this issue is installed. So once users install this update, the next update should be fine but I'm not 100% sure.

JoshuaLuckers commented 1 year ago

I recently did an update and it worked fine.

rthrash commented 1 year ago

I think this is resolved in one of the last few recent versions. Working again for me, too.

jcdm commented 1 year ago

I've just performed an upgrade and had the failure listed above (going TO 3.1.2-pl - not sure what I was coming FROM, but it did already match bezumkin's code above). Deactivating the existing plugin and then installing the update worked.