php-http / discovery

Finds installed clients and message factories
http://php-http.org
MIT License
1.27k stars 49 forks source link

Skip auto-installing when the root package's extra.discovery is enough #237

Closed nicolas-grekas closed 1 year ago

nicolas-grekas commented 1 year ago

Here is an idea that popped up in #236 and that builds on #232.

When the root package configures this:

    "extra": {
        "discovery": {
            "psr/http-factory-implementation": "My\\Psr17Factory"
        }
    }

then we shouldn't need auto-installing any extra packages for this virtual package.

Same when listing individual interfaces:

    "extra": {
        "discovery": {
            "Psr\\Http\\Client\\ClientInterface": "My\\Psr18Client"
        }
    }

In this case, we should verify that all the interfaces required to fulfill one of the supported *-implementation are listed.

dbu commented 1 year ago

thanks for the discussion in #236 . a workaround for now would be to do the composer config allow-plugins.php-http/discovery false from the readme, correct? and have the internal package provide the required *-implemenation information for composer.

I agree we can't check just for the *-implementation of installed packages because that still does not tell which classes to use. and scanning the code for implementing interfaces is too fragile (and a performance problem when no caching mechanism is available). while afaik java does support this sort of self-registration and autodiscovery mechanisms, i agree with not going that far.

the change suggested here would be that the existing configuration to chose which class to use for which psr factory also accepts unknown classes in addition to the well-known ones, right? that seems consistent with what we have and non-surprising. (btw there is some doc that is only in the readme but missing from https://docs.php-http.org/en/latest/discovery.html, we should sync the 2 places when we do this change)

nicolas-grekas commented 1 year ago

composer config allow-plugins.php-http/discovery false

correct!

the existing configuration to chose which class to use for which psr factory also accepts unknown classes in addition to the well-known ones

This works already! The only thing missing is skipping the installation of well-know packages when all interfaces of the virtual package are listed.

dbu commented 1 year ago

oh even better. yeah then its not much left to be done :+1:

and i just realized that you just added the pinning recently. neat, thanks!

nicolas-grekas commented 1 year ago

Implemented in #239