This extension causes major compatibly issues

[Nyholm]

I do appreciate all OS contributions and I really don't want to discourage others work. But I need to inform you that there is a huge problem when people are using this extension.

Since the code is loaded with PHP, it will avoid all composer version negotiation. Which means a user will always have psr/container:X and when they download a package that only supports psr/container:Y composer will not complain but the user will get an error at runtime.

I see that you have a section in the readme to inform users about this problem but it has two problems:

1. Not all users add that to their composer.json
2. It is not 100% correct.

I see a lot of confused users reporting issues they get after installing libraries that I maintain.

Example:

Fatal error: Declaration of Symfony\Component\DependencyInjection\ServiceLocator::has(string $id) must be compatible with Psr\Container\ContainerInterface::has($id) in vendor/symfony/dependency-injection/ServiceLocator.php on line 46

I strongly suggest this should be the top priority to fix. Especially since the problem will get way worse with #87.

[jbboehr]

When I first developed the extension, I was under the impression that any B/C break to the PSR interfaces would be published as a new PSR in a different namespace. It looks like they recently amended their bylaws or w/e and allowed changes to the interfaces. Needless to say, this is a huge compatibility nightmare for me. Sadly, it looks like I missed the opportunity to chime in [0], not that they would have changed their plans just for me.

The segment "usage with composer" is not necessary. The only thing it does it prevent the download of the PSR libraries. If the extension is loaded, PHP will always use the extension classes over the ones installed by composer.

Unfortunately, there isn't a whole lot I can do besides try to contact distribution maintainers and request that they do not enable the extension by default, unless phalcon is enabled. Unless you have any great ideas.

[jbboehr]

It is not 100% correct.

Can you elaborate?

[Nyholm]

It is not 100% correct.

Can you elaborate?

It does not consider PHP versions nor minor versions. This extension is effectively treating all PSR as they were the same version.

Unfortunately, there isn't a whole lot I can do besides try to contact distribution maintainers and request that they do not enable the extension by default, unless phalcon is enabled. Unless you have any great ideas.

Since this extension is causing massive issues and there are little to no real benefit of using it. My recommendation would be to remove it from pecl and archive the repository. It hurts me a little to say that, but given the changed state of PHP-Fig, it is both the simplest and most efficient way to move forward.

[sergeyklay]

Since this extension is causing massive issues and there are little to no real benefit of using it. My recommendation would be to remove it from pecl and archive the repository. It hurts me a little to say that, but given the changed state of PHP-Fig, it is both the simplest and most efficient way to move forward.

Are you suggesting hiding the issue instead of solving it? Did I understand you correctly?

[Nyholm]

No, not "hiding" the issue. I suggest that this extension should be removed.

Im happy to evaluate other suggestions too, but continue with the current path is a no go in my opinion. Users and maintainers are spending hours debugging problems caused by this extension.

Here are two issues in the last few weeks:

• Declaration of Symfony\Component\DependencyInjection\ServiceLocator::has(string $id) must be compatible with Psr\Container\ContainerInterface::has($id) symfony/symfony#40870
• fatal error after update 5.3 symfony/symfony#40393

---

I know that this is not the intention. I know that you spent a lot of time working on this. I am super impressed by your programming skills. But this is the situation we are in now and I fear it will get worse with #87 and the fact that libraries start to support the updated PSRs.

[sergeyklay]

Could someone explain to me: Can users just not install the extension? I'm sorry, but it seems I missed this place when we started saying that the only alternative is to delete the project. In my opinion, open source does not work like that, isn't?

[Nyholm]

Can users just not install the extension?

That is currently not what you do. Im doubtful that it will help...

I'm sorry, but it seems I missed this place when we started saying that the only alternative is to delete the project. In my opinion, open source does not work like that, isn't?

Do you have any suggestions how to solve the issue?

[sergeyklay]

Yes, I have. My suggestion is to intelligently and consciously install only those dependencies that you need. I would not want to create an unprecedented case of deleting one project due to the fact that this will be convenient in another project. You can't close your eyes to the the package manager issues mentioned above, or B/C in PSR interfaces mentioned above, or sloppy dependency management just by saying "this extension should be removed". Sorry, this is not how it works :)

[Nyholm]

My suggestion is to intelligently and consciously install only those dependencies that you need.

This is clearly not working. Hence the situation we are in now.

You can't close your eyes to the the package manager issues mentioned above, or B/C in PSR interfaces mentioned above, or sloppy dependency management just by saying "this extension should be removed".

Im not "closing my eyes". I opened this issue to take action. I also want to encourage the contributors and maintainers of this extension to take action.

---

What other suggestions are there to address this issue?

[jbboehr]

My recommendation would be to remove it from pecl and archive the repository.

This is not going to happen. I suspect once devilbox disables it by default there will be much fewer significant issues. No other distribution I'm aware of enables it by default, and if they do please tell me so I can request they do not.

In any case, I have had two ideas, neither of which I'm a huge fan of and require further testing to make sure they are feasible, but may help with the issue:

1. We can rename the interfaces provided by the extension to, for example, PsrExt and define a class_alias against their PSR interface name. The class_alias function is technically not allowed to be used on internal classes, but we can bypass this restriction since we are an extension.

This code for example:

<?php

namespace PsrExt\Log;

interface LoggerInterface {
  public function log(): void;
}


class_alias('PsrExt\Log\LoggerInterface', 'Psr\Log\LoggerInterface');

namespace Example;

class C implements \Psr\Log\LoggerInterface {

}

produces the following error message when run (note the class name in the error message):

Fatal error: Class Example\C contains 1 abstract method and must therefore be declared abstract or implement the remaining methods (PsrExt\Log\LoggerInterface::log) in tmp.php on line 14

I'm a little concerned it may cause subtle issues, but I can't think of any off the top of my head and, well, our test suite is extensive.

2. It should be technically possible for us to ship multiple versions of each PSR in a single release. I was hoping to avoid this since it's a maintenance headache, and may cause some trouble for implementing extensions. This would most likely use the class alias feature above.

The main issue is, of course, detecting which version to actually provide. The only "perfect" solution I suppose would be to attempt to scan for a composer.lock in the directory hierarchy at runtime. This is a huge performance hit and well I'm not very interested in implementing it.

An alternative would be to have an INI setting to select the desired PSR version. This is decidedly not optimal, but would allow some flexibility for users without having to juggle system dependency versions.

[jbboehr]

A full example:

<?php

namespace NativePsrExt\Container;

interface ContainerInterface_v1 {
  public function has(string $k);
}

interface ContainerInterface_v2 {
  public function has(string $k): bool;
}

switch (ini_get('psr.container_version')) {
  default: case '2':
    class_alias('NativePsrExt\Container\ContainerInterface_v2', 'Psr\Container\ContainerInterface');
  break;
}

namespace Example;

class MyContainer implements \Psr\Container\ContainerInterface {

}

produces:

Fatal error: Class Example\MyContainer contains 1 abstract method and must therefore be declared abstract or implement the remaining methods (NativePsrExt\Container\ContainerInterface_v2::has) in tmp.php on line 21

[Nyholm]

My recommendation would be to remove it from pecl and archive the repository.

This is not going to happen.

I understand that you feel that way. That is why Im asking for suggestions for other ways to solve the problem.

I suspect once devilbox disables it by default there will be much fewer significant issues. No other distribution I'm aware of enables it by default, and if they do please tell me so I can request they do not.

I really hope so.

In any case, I have had two ideas, neither of which I'm a huge fan of...

Hm. About your first suggestion. Wouldn't that cause the same issue?

About the second suggestion; it seams like I can toggle what PSR interface and version I want to use with an ini-setting. That is sure complicated but it make a little more sense. But it still bypass composers version negotiation.

---

Could you help me understand two things?

1. I see that this extension is needed for Phalcon, but I dont see why. Is it because some Phalcon code is implementing these interfaces and you want to keep in in C?
2. Does this extension bring any real benefit for users not using Phalcon?

[jbboehr]

Hm. About your first suggestion. Wouldn't that cause the same issue?

Yes, however the class name in the error message will be different and hopefully the user will realize the problem on their own, or when they file an issue it will be immediately apparent what the issue is.

About the second suggestion; it seams like I can toggle what PSR interface and version I want to use with an ini-setting. That is sure complicated but it make a little more sense. But it still bypass composers version negotiation.

Yes, although theoretically you could add some tooling to select the right versions based on what is installed by composer. It would have to be defined at the system level, not per-directory, as the classes are loaded at server startup. It could also be used to disable one or more of the conflicting PSRs and leave the others enabled, although if an extension were to depend on a disabled one, it it would cause runtime errors.

I see that this extension is needed for Phalcon, but I dont see why. Is it because some Phalcon code is implementing these interfaces and you want to keep in in C?

Extensions can only implement interfaces that are defined in native code. This is due to the fact that extension classes exist during the server lifecycle (MINIT) and not the request lifecycle (RINIT).

Phalcon is a framework, so they implement some of the interfaces.

And it's not useful just to phalcon. This extension provides a PSR7 implementation and requires this extension. It's a minor usage, but my other extension implements LoggerAwareInterface when this extension is loaded (for autowiring). This extension could use my extension to actually implement the PSR-3 interfaces, but they don't (they advertise as PSR-3 compatible but they are not as they don't actually implement the interfaces).

Does this extension bring any real benefit for users not using Phalcon?

Not really, although like I said earlier, it is useful for more extensions than just phalcon. It does cut down on classes loaded per request (yes I know how opcache works). But does this question doesn't really matter? They just don't need to use it.

[Nyholm]

Yes, however the class name in the error message will be different and hopefully the user will realize the problem on their own, or when they file an issue it will be immediately apparent what the issue is.

Oh, that is right. Smart!

yes I know how opcache works

I am confident that you (all contributors) know way more about the PHP engine than I do. =)

But does this question doesn't really matter? They just don't need to use it.

Im just trying to understand.

---

I do think the option 1 (#89) is a good way forward. It will not solve the problem that it composer cannot negotiate versions, but it will give the user/maintainers a clue to solve the problem much faster.

This together with devilbox removes the extension would probably avoid the situation in the future.

Big thank you!

[olivier-monaco]

Hi,

I've just faced out a compatiblity problem under PHP 7.0 via this extension. Some problem of type hints added to parameters of PSR-11 interfaces.

Adding such type hints follows the recommendations : https://www.php-fig.org/blog/2019/10/upgrading-psr-interfaces/

But, you provide the same source code for all PHP version, starting from PHP 7.0. But as PHP-FIG says, the libraries providing type hints must be compatible with PHP 7.2 or above.

I think the main problem comes from the PHP version supported by this extension. Starting with 1.1.1 (or maybe before), PHP version has to by 7.2 or above.

I hoep this helps.

[fisharebest]

Could someone explain to me: Can users just not install the extension?

A large webhost (https://ubserspace.de) installs this module, and provides no way to disable it.

The result is that many popular applications cannot be installed there.

[williamdes]

I did not read all the thread but just the start and by chance I did open an issue for phpMyAdmin some time ago

suspect once devilbox disables it by default

See: devilbox/docker-php-fpm#201