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

Fix issue with opcache preload and class_alias #129

Closed
wants to merge 1 commit into from
Closed

Fix issue with opcache preload and class_alias #129

wants to merge 1 commit into from

Conversation

starred-gijs
Copy link

Fixes #128

Q A
Bugfix yes
BC Break no
New Feature no
RFC no
QA no

Description

Not sure how to test this, except the manual tests I ran.

Signed-off-by: Gijs van Lammeren <gijs@starred.com>
@boesing
Copy link
Member

boesing commented Apr 26, 2022

The problem is, that the whole idea of replacing container-interop is to replace it from within THIS repository.
If we now allow other interfaces to replace container-interop, the whole idea of why we made this change won't work anymore. Because your application will break for sure in case that the Interop\ContainerInterop\ContainerInterface is not exactly the same interface as Psr\Container\ContainerInterface.


So do I understand correct, that calling class_alias in an opcached application breaks when this was done after the initial loading of that script?

When we do add interface_exists here, we should at least verify that the interface which does exist is pointing the expected PSR-11 interface.

Something like this (which does obv. not work, but I don't know how to verify that):

interface Foo
{}
if (!interface_exists(Bar::class)) {
     class_alias(Foo::class, 'Bar');
}

assert(Bar::class === Foo::class);

Because silently allow projects to have an existing implementation of the interop interface might break projects as we changed the method type-hints to require PSR-11 implementations.

(Is it clear what I want to point out? I am not sure if that was clear...)

@Ocramius
Copy link
Member

That's kinda why I wanted to test this: I think this might be a limitation of preloading, which only stores the declarations.

This pull request was closed.
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

Successfully merging this pull request may close these issues.

Cannot declare interface Interop\Container\Exception\NotFoundException, because the name is already in use
3 participants