cedaro / satispress

Expose installed WordPress plugins and themes as Composer packages.
500 stars 48 forks source link

Compile error with v1.0.3 #171

Open E-VANCE opened 2 years ago

E-VANCE commented 2 years ago

Hi there,

first of all: Awesome plugin, solved so many headaches for me. Many thanks for developing and maintaining 🚀.

I ran into an issue when SatisPress (supposedly) updated to 1.0.3 – at least I could resolve it by downgrading to 1.0.2 and the error is re-producible in my environment.

This is the error output:

E_COMPILE_ERROR in line 59 of /wp-content/plugins/satispress/vendor/pimple/pimple/src/Pimple/Psr11/ServiceLocator.php – declaration of Pimple\Psr11\ServiceLocator::get(string $id) must be compatible with Psr\Container\ContainerInterface::get($id)

Running PHP 7.4.26 on Apache 2.4.51 with WordPress 5.8.3. Let me know if you need more specific debugging information.

Do you have an idea what could be causing this issue...?

Thanks & regards, Henning

bradyvercher commented 2 years ago

@E-VANCE Thanks! I'm glad to hear SatisPress has been helpful for you.

How are you installing SatisPress (Composer or the release zip?). It looks like an older version of psr/container is being installed that isn't compatible with the version of the Pimple container that's being installed.

E-VANCE commented 2 years ago

I used the release zip since the installation that serves SatisPress is not handled via Composer...

Just checked the 1.0.3 release zip files and those seem OK. Strangely enough the error is being thrown again upon upgrading from 1.0.2.

Not sure how to proceed...?

bradyvercher commented 2 years ago

Do you have another active plugin that's loading an older version of psr/container?

E-VANCE commented 2 years ago

Good question... I'll go ahead and deactivate all other plugins, upgrade to 1.0.3 and report back.

E-VANCE commented 2 years ago

Gotcha...

iThemes Security Pro in version 7.0.3 also includes Pimple\Psr11 which references / uses Psr\Container\ContainerInterface and throws this error. Guess it takes precedence and then generates this conflict...

The error message given isn't very helpful to this regard 😐

I'll keep the security plugin deactivated for now, they'll probably update their dependencies at some point (and should, the last plugin update is from August 2021).

Or do you see a sensible way of checking for already included packages...?

Anyhow, let's get back to work on more important stuff 😉

Thanks for your hint and keep it up!

lkraav commented 2 years ago

Oof, this is real. And I can't really disable iThemes Security Pro.

https://github.com/woocommerce/woocommerce/issues/27384 recently went through similar, and there are tools that let you auto-rename certain namespaces.

From https://github.com/woocommerce/woocommerce/issues/27384#issuecomment-675606701

https://github.com/humbug/php-scoper https://github.com/coenjacobs/mozart https://github.com/TypistTech/imposter-plugin

bradyvercher commented 2 years ago

This kind of the bummer with relying on dependencies in WordPress. SatisPress or iThemes could prefix the dependencies, but things like psr/container are meant to be a global interface used across dependencies.

If I lock the versions used in SatisPress, then we'd run into the same issue next time versions became mismatched in any plugin.

I have used Mozart in the past. While it'd be a breaking change and bypass the benefits of using global interfaces like psr/container and psr/log, something like that might be the only viable option.

Strauss is another option that looks to be an updated version of Mozart.

lkraav commented 2 years ago

This kind of the bummer with relying on dependencies in WordPress. SatisPress or iThemes could prefix the dependencies, but things like psr/container are meant to be a global interface used across dependencies. If I lock the versions used in SatisPress, then we'd run into the same issue next time versions became mismatched in any plugin. I have used Mozart in the past. While it'd be a breaking change and bypass the benefits of using global interfaces like psr/container and psr/log, something like that might be the only viable option.

If bandwidth, check how Action Scheduler currently handles getting loaded from multiple packages across WooCommerce ecosystem (and now other things use it for background jobs, too).

But that's a WP-specific package, and generic PHP libraries may not feasible to patch for such.

All discussions I've seen about this very problem have concluded "global interface used across dependencies" just doesn't work out for time being, not until composer becomes the central package manager for WP, and that may literally never happen.

I think doing namespace juggling is the only sane way forward.